[BUILD FAILED] Branch master build 659

[Apache Jenkins Server]

Build for MXNet branch master has broken. Please view the build at 
https://builds.apache.org/job/incubator-mxnet/job/master/659/

Re: [Important] Please Help make the Apache MXNet (incubating) 1.0 Release Notes Better!

[Pedro Larroy]

Thank you Meghna

Added notes about ARM & Nvidia Jetson support (beta) to the document.

On Mon, Nov 20, 2017 at 2:19 PM, Meghna Baijal
 wrote:
> Apologies. Done.
>
> On Mon, Nov 20, 2017 at 2:18 PM, Chris Olivier 
> wrote:
>
>> No write access :(
>>
>> On Mon, Nov 20, 2017 at 2:16 PM, Meghna Baijal > >
>> wrote:
>>
>> > Hello All,
>> > As you know I am currently working on finalizing the Release Candidate
>> for
>> > Apache MXNet (incubating) 1.0 Release. Anyone who has contributed to this
>> > release, could you please go through the release notes in the shared doc
>> > linked below and review/make changes as needed.
>> >
>> > Link -
>> > https://docs.google.com/document/d/1SdFwiTXlFBMmyVfEHpe7s3jteWxqf
>> > UzsxTgbagcZWzo/edit?usp=sharing
>> >
>> > The notes are very limited in details so feel free to add any
>> details/links
>> > to tutorials or documentation that you think will be useful. Changes can
>> be
>> > made until *EOD tonight (Monday, 11/20)* after which they need to be
>> merged
>> > into the NEWS.md.
>> >
>> > Thanks,
>> > Meghna Baijal
>> >
>>

Re: 3rdparty packages as submodules

[Pedro Larroy]

We could also add gtest as well for example.



I would like to point out that is quite cumbersome to get your code
tested and ready before sending a PR, this includes installing
cpplint, pylint, gtest…

Installing gtest and bootstrapping it is not completely trivial.



Kind regards.

On Mon, Nov 20, 2017 at 11:23 AM, Eric Xie  wrote:
> I'm fine with a 3rdparty folder. Not sure about apache legal.
>
> On 2017-11-17 10:25, Chris Olivier  wrote:
>> All,
>>
>> I often find it desirable to have a method for 3rdparty packages to be
>> included (possibly optionally) in a 3rdparty directory.   We do this with
>> 'cub' to some degree, but it's in the root and is actually a fork in the
>> dmlc repository.  Some samples of what might go in there:
>>
>> 1) Intel OpenMP (llvm-openmp) -- In order to use Intel OMP by default
>> 2) gperftools -- In order to build statically with -fPIC, which isn't the
>> case with the general distribution
>> 3) mkl-dnn -- In order to build and have debug information available for
>> mkl-dnn (and possibly submit bugfixes)
>>
>> What do you all think?
>>
>> -Chris
>>

Re: [RFQ] Deprecate amalgamation

[Pedro Larroy]

I like the idea of amalgamation, I have used it in SQLite as it makes
very easy to just drop one header file and one source file in another
project to use the library.

But SQLite is often used as a library embedded in platforms / other libraries.

What's the use case of amalgamation in MXNet when we can build the
binary library for all the platforms with MXNet's build system?  Who
is using MXNet as an embedded library that can't use the shared
library + headers or specific language bindings?

Can't we call emscripten from CMake? I'm not familiar with our JS
bindings, but I don't see why we can't compile for emscripten as for
any other platform.

Pedro.

On Mon, Nov 20, 2017 at 11:59 PM, Tianqi Chen  wrote:
> We could resort to a middle ground. Instead of having an amalgamation
> script that generates a single file, simply have a file that includes
> everything and compiles that one. Which should also work.
>
> The javascript port can likely be superseded with some form of support in
> nnvm compiler, which transpires and generate likely more efficient code
> than current version.  We can enable that feature now except that there is
> no dedicated developer on it yet. We can talk about full deprecation after
> this
>
>
> Tianqi
>
> On Mon, Nov 20, 2017 at 2:47 PM, Pedro Larroy 
> wrote:
>
>> Hi all
>>
>> Given that we have working builds for ARM, Android, TX2 and the main
>> architectures, and after considering how amalgamation is done. I would
>> like to propose that we deprecate and remove amalgamation.
>>
>> I don't think the cost of maintaining this feature and how it's done
>> justifies the ROI, given that we can now produce binary builds for
>> embedded platforms in a comfortable way. It's also consuming build &
>> test resources.
>>
>> We should strive to simplify our build system and development process.
>>
>> Pedro.
>>

Re: Protected master needs to be turned off

[kellen sunderland]

-0.5 (non-binding).   I would propose that if there are pieces of CI that
are slowing down development (and I think we can all agree that there are)
that we should strip out these problematic CI pieces and open issues for
them.  We can then assign issues to people and evaluate what should be done
to fix or mitigate the root cause of the problems (be it slow runtime,
flakiness, etc.).  This way we will get to a state where the CI is stable,
and we'll have a backlog of issues to fix.

-Kellen

On Mon, Nov 20, 2017 at 2:36 PM, Marco de Abreu <
marco.g.ab...@googlemail.com> wrote:

> Small update regarding the status of the new CI: We've been able to get
> ubuntu builds and tests up and running.
>
> I’ve received requests to provide exact times for all important stages of
> the CI. As the first runs have been executed successfully, I’ll provide you
> with some data. This will give you a small guidance where our bottlenecks
> are located and in which field we can have improvements. All jobs of the
> same stage are being executed in parallel.
>
>
>
> In the following you’ll see the durations of all Ubuntu-Builds and -Tests.
> They are all (CPU- and GPU-tests) executed on a G3.8xlarge with 32 vCPUs.
> These times only cover the actual core execution time of the task
> *without* stashing,
> cleaning etc.
>
>
>
> Build:
>
>- Amalgamation: 3m40s
>- Amalgamation MIN: 3m49s
>- *CPU: Openblas: 8m38s*
>- *GPU CUDA7.5+cuDNN5: 11m55s*
>- *GPU: MKLML: 9m42s*
>
>
>
>
>
> Unit Test:
>
>- Perl CPU: 17m36s
>- Perl GPU: 8m31s
>- *Python2: CPU: 1h10m10s*
>- Python2: GPU: 14m10s
>- Python2: MKLML-CPU: 10m25s + 1m43s = 12m08s
>- Python2: MKLML-GPU: 14m19s
>- *Python3: CPU: 1h8m26s*
>- Python3: GPU: 14m58s
>- Python3: MKLML-CPU: 10m31s (seems like this job is not running the
>Python2-Train-Tests)
>- Python3: MKLML-GPU: 13m37s
>- *R: CPU: 20m6s + 9s + 9m58s = 30m13s*
>- *R: GPU: 21m50s + 10s + 1m17s = 23m17s*
>- Scala: CPU: 2m18s + 3m57s = 6m15s
>
>
>
> Integration Test:
>
>- Caffe GPU: 7m42s
>- Python GPU: 1m47s
>- Cpp-package GPU: 59s
>
>
> The next step is to get Windows up and running and improve execution time
> on CPU-tasks.
>
> -Marco
>
> On Mon, Nov 20, 2017 at 9:18 PM, YiZhi Liu  wrote:
>
> > +1
> >
> > 2017-11-20 11:47 GMT-08:00 Tianqi Chen :
> > > +1 until new CI is implemented.
> > >
> > > Tianqi
> > >
> > > On Mon, Nov 20, 2017 at 11:11 AM, Eric Xie  wrote:
> > >
> > >> A lot of people seems to be confused, so let's clarify the separation
> of
> > >> roles/responsibilities:
> > >>
> > >> 1. The committers that merge code are responsible for code quality and
> > >> tests passing on master.
> > >>
> > >> 2. The CI / infra maintainers are responsible for keeping CI running
> > >> properly and honestly reporting bugs.
> > >> If test fails because Jenkins is faulty or slow, you need to fix
> > Jenkins.
> > >> If test fails because there is a bug in the code, then you did a good
> > job
> > >> catching bugs. It is not your responsibility to fix the bug. You
> merely
> > >> should report it to committers.
> > >>
> > >> Thanks,
> > >> Junyuan Xie
> > >>
> > >>
> > >> On 2017-11-19 16:07, Gautam  wrote:
> > >> > -1
> > >> >
> > >> > Please see inline.
> > >> >
> > >> > On Nov 19, 2017 12:51 PM, "Eric Xie"  wrote:
> > >> >
> > >> > Hi all,
> > >> > I'm starting this thread to vote on turning off protected master.
> The
> > >> > reasons are:
> > >> >
> > >> > 1. Since we turned on protected master pending PRs has grown from 40
> > to
> > >> 80.
> > >> > It is severely slowing down development.
> > >> >
> > >> >
> > >> >  Turning protection off, will give you ability to merge the code
> > >> which
> > >> > has build failure. How and more importantly who will be best judge
> to
> > >> > figure out what's is wrong ? If CI is a problem then we have people
> > from
> > >> > infra team, who are sort of 'on call" day and night trying to fix.
> > >> > Including me trying to fix the slave at 2 am in night. I have seen
> > >> > commiters sending PRs and if it fails without even looking at the
> > >> > reason,which could be just a temporary error, they reach out to
> infra
> > >> team
> > >> > immediately. So I don't agree that we should turn off the protected
> > >> branch
> > >> > just for the sake of speed. We should care more on quality than
> speed.
> > >> >
> > >> >
> > >> > 2. Committers, not CI, are ultimately responsible for the code they
> > >> merge.
> > >> > You should only override the CI when you are very confident that CI
> is
> > >> the
> > >> > problem, not your code. If it turns out you are wrong, you should
> fix
> > it
> > >> > ASAP. This is the bare minimum requirement for all committers: BE
> > >> > RESPONSIBLE.
> > >> >
> > >> > How do we make sure that this in deed happens?
> > >> >
> > >> > I'm aware of the argument for 

Re: [RFQ] Deprecate amalgamation

[Tianqi Chen]

We could resort to a middle ground. Instead of having an amalgamation
script that generates a single file, simply have a file that includes
everything and compiles that one. Which should also work.

The javascript port can likely be superseded with some form of support in
nnvm compiler, which transpires and generate likely more efficient code
than current version.  We can enable that feature now except that there is
no dedicated developer on it yet. We can talk about full deprecation after
this


Tianqi

On Mon, Nov 20, 2017 at 2:47 PM, Pedro Larroy 
wrote:

> Hi all
>
> Given that we have working builds for ARM, Android, TX2 and the main
> architectures, and after considering how amalgamation is done. I would
> like to propose that we deprecate and remove amalgamation.
>
> I don't think the cost of maintaining this feature and how it's done
> justifies the ROI, given that we can now produce binary builds for
> embedded platforms in a comfortable way. It's also consuming build &
> test resources.
>
> We should strive to simplify our build system and development process.
>
> Pedro.
>

Re: [RFQ] Deprecate amalgamation

[Khanna, Aran]

What about the Web/Node inference build MXnet JS 
(https://github.com/apache/incubator-mxnet/tree/master/amalgamation#javascript 
)?

Best,
Aran 

On 11/20/17, 2:47 PM, "Pedro Larroy"  wrote:

Hi all

Given that we have working builds for ARM, Android, TX2 and the main
architectures, and after considering how amalgamation is done. I would
like to propose that we deprecate and remove amalgamation.

I don't think the cost of maintaining this feature and how it's done
justifies the ROI, given that we can now produce binary builds for
embedded platforms in a comfortable way. It's also consuming build &
test resources.

We should strive to simplify our build system and development process.

Pedro.

Re: [RFQ] Deprecate amalgamation

[Chris Olivier]

Is amalgamation was used for the javascript port?

On Mon, Nov 20, 2017 at 2:47 PM, Pedro Larroy 
wrote:

> Hi all
>
> Given that we have working builds for ARM, Android, TX2 and the main
> architectures, and after considering how amalgamation is done. I would
> like to propose that we deprecate and remove amalgamation.
>
> I don't think the cost of maintaining this feature and how it's done
> justifies the ROI, given that we can now produce binary builds for
> embedded platforms in a comfortable way. It's also consuming build &
> test resources.
>
> We should strive to simplify our build system and development process.
>
> Pedro.
>

[RFQ] Deprecate amalgamation

[Pedro Larroy]

Hi all

Given that we have working builds for ARM, Android, TX2 and the main
architectures, and after considering how amalgamation is done. I would
like to propose that we deprecate and remove amalgamation.

I don't think the cost of maintaining this feature and how it's done
justifies the ROI, given that we can now produce binary builds for
embedded platforms in a comfortable way. It's also consuming build &
test resources.

We should strive to simplify our build system and development process.

Pedro.

Re: Protected master needs to be turned off

[Marco de Abreu]

Small update regarding the status of the new CI: We've been able to get
ubuntu builds and tests up and running.

I’ve received requests to provide exact times for all important stages of
the CI. As the first runs have been executed successfully, I’ll provide you
with some data. This will give you a small guidance where our bottlenecks
are located and in which field we can have improvements. All jobs of the
same stage are being executed in parallel.



In the following you’ll see the durations of all Ubuntu-Builds and -Tests.
They are all (CPU- and GPU-tests) executed on a G3.8xlarge with 32 vCPUs.
These times only cover the actual core execution time of the task
*without* stashing,
cleaning etc.



Build:

   - Amalgamation: 3m40s
   - Amalgamation MIN: 3m49s
   - *CPU: Openblas: 8m38s*
   - *GPU CUDA7.5+cuDNN5: 11m55s*
   - *GPU: MKLML: 9m42s*





Unit Test:

   - Perl CPU: 17m36s
   - Perl GPU: 8m31s
   - *Python2: CPU: 1h10m10s*
   - Python2: GPU: 14m10s
   - Python2: MKLML-CPU: 10m25s + 1m43s = 12m08s
   - Python2: MKLML-GPU: 14m19s
   - *Python3: CPU: 1h8m26s*
   - Python3: GPU: 14m58s
   - Python3: MKLML-CPU: 10m31s (seems like this job is not running the
   Python2-Train-Tests)
   - Python3: MKLML-GPU: 13m37s
   - *R: CPU: 20m6s + 9s + 9m58s = 30m13s*
   - *R: GPU: 21m50s + 10s + 1m17s = 23m17s*
   - Scala: CPU: 2m18s + 3m57s = 6m15s



Integration Test:

   - Caffe GPU: 7m42s
   - Python GPU: 1m47s
   - Cpp-package GPU: 59s


The next step is to get Windows up and running and improve execution time
on CPU-tasks.

-Marco

On Mon, Nov 20, 2017 at 9:18 PM, YiZhi Liu  wrote:

> +1
>
> 2017-11-20 11:47 GMT-08:00 Tianqi Chen :
> > +1 until new CI is implemented.
> >
> > Tianqi
> >
> > On Mon, Nov 20, 2017 at 11:11 AM, Eric Xie  wrote:
> >
> >> A lot of people seems to be confused, so let's clarify the separation of
> >> roles/responsibilities:
> >>
> >> 1. The committers that merge code are responsible for code quality and
> >> tests passing on master.
> >>
> >> 2. The CI / infra maintainers are responsible for keeping CI running
> >> properly and honestly reporting bugs.
> >> If test fails because Jenkins is faulty or slow, you need to fix
> Jenkins.
> >> If test fails because there is a bug in the code, then you did a good
> job
> >> catching bugs. It is not your responsibility to fix the bug. You merely
> >> should report it to committers.
> >>
> >> Thanks,
> >> Junyuan Xie
> >>
> >>
> >> On 2017-11-19 16:07, Gautam  wrote:
> >> > -1
> >> >
> >> > Please see inline.
> >> >
> >> > On Nov 19, 2017 12:51 PM, "Eric Xie"  wrote:
> >> >
> >> > Hi all,
> >> > I'm starting this thread to vote on turning off protected master. The
> >> > reasons are:
> >> >
> >> > 1. Since we turned on protected master pending PRs has grown from 40
> to
> >> 80.
> >> > It is severely slowing down development.
> >> >
> >> >
> >> >  Turning protection off, will give you ability to merge the code
> >> which
> >> > has build failure. How and more importantly who will be best judge to
> >> > figure out what's is wrong ? If CI is a problem then we have people
> from
> >> > infra team, who are sort of 'on call" day and night trying to fix.
> >> > Including me trying to fix the slave at 2 am in night. I have seen
> >> > commiters sending PRs and if it fails without even looking at the
> >> > reason,which could be just a temporary error, they reach out to infra
> >> team
> >> > immediately. So I don't agree that we should turn off the protected
> >> branch
> >> > just for the sake of speed. We should care more on quality than speed.
> >> >
> >> >
> >> > 2. Committers, not CI, are ultimately responsible for the code they
> >> merge.
> >> > You should only override the CI when you are very confident that CI is
> >> the
> >> > problem, not your code. If it turns out you are wrong, you should fix
> it
> >> > ASAP. This is the bare minimum requirement for all committers: BE
> >> > RESPONSIBLE.
> >> >
> >> > How do we make sure that this in deed happens?
> >> >
> >> > I'm aware of the argument for using protected master: It make sure
> that
> >>
> >>
> >>
> >> > master is stable.
> >> >
> >> > Well, master will be most stable if we stop adding any commits to it.
> But
> >> > that's not what we want is it?
> >> >
> >> > No we are not saying don't add anything in master, we are just saying
> >> > please don't add bad code to the master. And yes I have seen bad code
> has
> >> > been merged to the master when protected branch was not enabled.
> >> >
> >> > Protected master hardly adds any stability. The faulty tests that
> breaks
> >> > master at random got merged into master because they happened to
> succeed
> >> > once.
> >> >
> >> > That's not true, it filter out one of the important aspect that at
> least
> >> > when code was merged it completed the whole cycle of build and test.
> Sure
> >> > flaky test we can track down.
> >> >
> 

Re: [Important] Please Help make the Apache MXNet (incubating) 1.0 Release Notes Better!

[Meghna Baijal]

Apologies. Done.

On Mon, Nov 20, 2017 at 2:18 PM, Chris Olivier 
wrote:

> No write access :(
>
> On Mon, Nov 20, 2017 at 2:16 PM, Meghna Baijal  >
> wrote:
>
> > Hello All,
> > As you know I am currently working on finalizing the Release Candidate
> for
> > Apache MXNet (incubating) 1.0 Release. Anyone who has contributed to this
> > release, could you please go through the release notes in the shared doc
> > linked below and review/make changes as needed.
> >
> > Link -
> > https://docs.google.com/document/d/1SdFwiTXlFBMmyVfEHpe7s3jteWxqf
> > UzsxTgbagcZWzo/edit?usp=sharing
> >
> > The notes are very limited in details so feel free to add any
> details/links
> > to tutorials or documentation that you think will be useful. Changes can
> be
> > made until *EOD tonight (Monday, 11/20)* after which they need to be
> merged
> > into the NEWS.md.
> >
> > Thanks,
> > Meghna Baijal
> >
>

Re: [Important] Please Help make the Apache MXNet (incubating) 1.0 Release Notes Better!

[Chris Olivier]

No write access :(

On Mon, Nov 20, 2017 at 2:16 PM, Meghna Baijal 
wrote:

> Hello All,
> As you know I am currently working on finalizing the Release Candidate for
> Apache MXNet (incubating) 1.0 Release. Anyone who has contributed to this
> release, could you please go through the release notes in the shared doc
> linked below and review/make changes as needed.
>
> Link -
> https://docs.google.com/document/d/1SdFwiTXlFBMmyVfEHpe7s3jteWxqf
> UzsxTgbagcZWzo/edit?usp=sharing
>
> The notes are very limited in details so feel free to add any details/links
> to tutorials or documentation that you think will be useful. Changes can be
> made until *EOD tonight (Monday, 11/20)* after which they need to be merged
> into the NEWS.md.
>
> Thanks,
> Meghna Baijal
>

Re: Protected master needs to be turned off

[Tianqi Chen]

+1 until new CI is implemented.

Tianqi

On Mon, Nov 20, 2017 at 11:11 AM, Eric Xie  wrote:

> A lot of people seems to be confused, so let's clarify the separation of
> roles/responsibilities:
>
> 1. The committers that merge code are responsible for code quality and
> tests passing on master.
>
> 2. The CI / infra maintainers are responsible for keeping CI running
> properly and honestly reporting bugs.
> If test fails because Jenkins is faulty or slow, you need to fix Jenkins.
> If test fails because there is a bug in the code, then you did a good job
> catching bugs. It is not your responsibility to fix the bug. You merely
> should report it to committers.
>
> Thanks,
> Junyuan Xie
>
>
> On 2017-11-19 16:07, Gautam  wrote:
> > -1
> >
> > Please see inline.
> >
> > On Nov 19, 2017 12:51 PM, "Eric Xie"  wrote:
> >
> > Hi all,
> > I'm starting this thread to vote on turning off protected master. The
> > reasons are:
> >
> > 1. Since we turned on protected master pending PRs has grown from 40 to
> 80.
> > It is severely slowing down development.
> >
> >
> >  Turning protection off, will give you ability to merge the code
> which
> > has build failure. How and more importantly who will be best judge to
> > figure out what's is wrong ? If CI is a problem then we have people from
> > infra team, who are sort of 'on call" day and night trying to fix.
> > Including me trying to fix the slave at 2 am in night. I have seen
> > commiters sending PRs and if it fails without even looking at the
> > reason,which could be just a temporary error, they reach out to infra
> team
> > immediately. So I don't agree that we should turn off the protected
> branch
> > just for the sake of speed. We should care more on quality than speed.
> >
> >
> > 2. Committers, not CI, are ultimately responsible for the code they
> merge.
> > You should only override the CI when you are very confident that CI is
> the
> > problem, not your code. If it turns out you are wrong, you should fix it
> > ASAP. This is the bare minimum requirement for all committers: BE
> > RESPONSIBLE.
> >
> > How do we make sure that this in deed happens?
> >
> > I'm aware of the argument for using protected master: It make sure that
>
>
>
> > master is stable.
> >
> > Well, master will be most stable if we stop adding any commits to it. But
> > that's not what we want is it?
> >
> > No we are not saying don't add anything in master, we are just saying
> > please don't add bad code to the master. And yes I have seen bad code has
> > been merged to the master when protected branch was not enabled.
> >
> > Protected master hardly adds any stability. The faulty tests that breaks
> > master at random got merged into master because they happened to succeed
> > once.
> >
> > That's not true, it filter out one of the important aspect that at least
> > when code was merged it completed the whole cycle of build and test. Sure
> > flaky test we can track down.
> >
> > Thanks,
> > Junyuan Xie
> >
>

Re: 3rdparty packages as submodules

[Eric Xie]

I'm fine with a 3rdparty folder. Not sure about apache legal.

On 2017-11-17 10:25, Chris Olivier  wrote: 
> All,
> 
> I often find it desirable to have a method for 3rdparty packages to be
> included (possibly optionally) in a 3rdparty directory.   We do this with
> 'cub' to some degree, but it's in the root and is actually a fork in the
> dmlc repository.  Some samples of what might go in there:
> 
> 1) Intel OpenMP (llvm-openmp) -- In order to use Intel OMP by default
> 2) gperftools -- In order to build statically with -fPIC, which isn't the
> case with the general distribution
> 3) mkl-dnn -- In order to build and have debug information available for
> mkl-dnn (and possibly submit bugfixes)
> 
> What do you all think?
> 
> -Chris
> 

Re: Protected master needs to be turned off

[Meghna Baijal]

-1 (non binding)
While I agree that the protected master is slowing development but it is
also helping identify problematic tests. If we don't do this now, the same
issues will exist on the new CI.

On Sun, Nov 19, 2017 at 1:51 PM, Marco de Abreu <
marco.g.ab...@googlemail.com> wrote:

> Hello,
>
> -1 (non binding)
>
> Who is going to be responsible for changes breaking tests and having other
> side effects after they have been merged? I'm afraid that this will harm
> further development. At the moment I'm the responsible person for setting
> up the new CI and so far have my results shown that not the CI itself is
> the problem but also the stability of our code as well as the tests
> themselves. At the moment we are having big issues to get a stable CI
> because MXNet seems to be relying on so specific architectures,
> dependencies and other factors which I'm not even able to track down that
> this causes everything to be unstable.
>
> Just to point it out: If we encounter so many problems while setting up a
> CI system, doesn't that mean that our users and customers are also going to
> face those issues as soon as things are getting more complicated? This is a
> red flag in my opinion and I'm really looking forward to the usability
> Sprint, but at the moment I'm afraid that an unprotected master will make
> the situation even worse. It's already enough work to isolate and fix the
> current issues, but if new untested changes get merged, this is going to be
> like fighting a wildfire with a bottle of water.
>
> So please revise your thoughts. If anybody is blocked by the protected
> master, I would really appreciate it if they could approach me personally
> in order to help stabilising the current situation. Just feeding in more
> and more changes on one end while we're fixing issues on the other end
> won't get us anywhere.
>
> Best regards,
> Marco
>
> Am 19.11.2017 10:08 nachm. schrieb "Chris Olivier"  >:
>
> > Revised:
> >
> >
> > +1 at least until new CI is implemented. Then reevaluate.
> >
> > On Sun, Nov 19, 2017 at 1:07 PM Chris Olivier 
> > wrote:
> >
> > > +1
> > >
> > >
> > > On Sun, Nov 19, 2017 at 12:52 PM Zha, Sheng 
> wrote:
> > >
> > >> +1
> > >>
> > >> Best regards,
> > >> -sz
> > >>
> > >> On 11/19/17, 12:51 PM, "Eric Xie"  wrote:
> > >>
> > >> Hi all,
> > >> I'm starting this thread to vote on turning off protected master.
> > The
> > >> reasons are:
> > >>
> > >> 1. Since we turned on protected master pending PRs has grown from
> 40
> > >> to 80. It is severely slowing down development.
> > >>
> > >> 2. Committers, not CI, are ultimately responsible for the code
> they
> > >> merge. You should only override the CI when you are very confident
> that
> > CI
> > >> is the problem, not your code. If it turns out you are wrong, you
> should
> > >> fix it ASAP. This is the bare minimum requirement for all committers:
> BE
> > >> RESPONSIBLE.
> > >>
> > >> I'm aware of the argument for using protected master: It make sure
> > >> that master is stable.
> > >>
> > >> Well, master will be most stable if we stop adding any commits to
> > it.
> > >> But that's not what we want is it?
> > >>
> > >> Protected master hardly adds any stability. The faulty tests that
> > >> breaks master at random got merged into master because they happened
> to
> > >> succeed once.
> > >>
> > >> Thanks,
> > >> Junyuan Xie
> > >>
> > >>
> > >>
> > >>
> >
>

Re: Protected master needs to be turned off

[Naveen Swamy]

-1
for the reasons, Gautam and Marco have pointed out.

On Sun, Nov 19, 2017 at 4:07 PM, Gautam  wrote:

> -1
>
> Please see inline.
>
> On Nov 19, 2017 12:51 PM, "Eric Xie"  wrote:
>
> Hi all,
> I'm starting this thread to vote on turning off protected master. The
> reasons are:
>
> 1. Since we turned on protected master pending PRs has grown from 40 to 80.
> It is severely slowing down development.
>
>
>  Turning protection off, will give you ability to merge the code which
> has build failure. How and more importantly who will be best judge to
> figure out what's is wrong ? If CI is a problem then we have people from
> infra team, who are sort of 'on call" day and night trying to fix.
> Including me trying to fix the slave at 2 am in night. I have seen
> commiters sending PRs and if it fails without even looking at the
> reason,which could be just a temporary error, they reach out to infra team
> immediately. So I don't agree that we should turn off the protected branch
> just for the sake of speed. We should care more on quality than speed.
>
>
> 2. Committers, not CI, are ultimately responsible for the code they merge.
> You should only override the CI when you are very confident that CI is the
> problem, not your code. If it turns out you are wrong, you should fix it
> ASAP. This is the bare minimum requirement for all committers: BE
> RESPONSIBLE.
>
> How do we make sure that this in deed happens?
>
> I'm aware of the argument for using protected master: It make sure that
> master is stable.
>
> Well, master will be most stable if we stop adding any commits to it. But
> that's not what we want is it?
>
> No we are not saying don't add anything in master, we are just saying
> please don't add bad code to the master. And yes I have seen bad code has
> been merged to the master when protected branch was not enabled.
>
> Protected master hardly adds any stability. The faulty tests that breaks
> master at random got merged into master because they happened to succeed
> once.
>
> That's not true, it filter out one of the important aspect that at least
> when code was merged it completed the whole cycle of build and test. Sure
> flaky test we can track down.
>
> Thanks,
> Junyuan Xie
>

Re: AWS contributing ONNX-MXNet

[Steffen Rochel]

We modified the blog post and acknowledged Zhi's contributions.
It reads now: "*Special thanks to the dmlc/nnvm community and Zhi Zhang,
whose ONNX code was used as a reference for this implementation."*

Regards,
Steffen

On Fri, Nov 17, 2017 at 10:36 AM Tianqi Chen 
wrote:

> I have watched the issue for around two days. Here are my two cents.
>
> First of all, there is no legal constraint to enforce you do anything, but
> as you said(which I fully agree on), we need to assume others have best
> intentions and give goodwill
>
> - It is  great to reuse code, that is what open-source is about
>
> - It is un-arguably true that Zhi created and maintained most of part of
> the original code. While there are minor contributions from other
> contributors. I think Zhi should be personally acknowledged at least(he
> deserve more than that).
>   - As an analogy,  you are not the only one creating the onnx-mxnet
> repo, but never the less you are listed as the author, instead of simply
> saying that comes from AWS
>
> - I would recommend you start with the files of nnvm as your first commit,
> then apply changes to it.
>  - This will take around 5 min or so, copy the file from nnvm, commit,
> override with your new file, commit
>  - It makes it clear what changes are being done
>  - It makes your life easier to adopt new patches when there is a
> bugfix in nnvm or vice versa
>
>
> - Please maintain it, instead of leaving the job to the community. As with
> every great prize comes with great responsibility,  it is great that you
> push out the repo and takes the credit for doing it. The deep learning
> serializable IR land is still unstable and there demand the efforts to put
> in to maintain the code to keep up with the breaking changes and add
> coverage.
>
> Congrats on the release
> Tianqi
>
>
>
> On Thu, Nov 16, 2017 at 2:04 PM, Lupesko, Hagay  wrote:
>
> > Hey folks,
> >
> >
> >
> > Today AWS announced contributing ONNX-MXNet, an open source Python
> package
> > that imports ONNX models into MXNet. @roshrini and I (@lupesko) have
> worked
> > on the code, which is now publicly available [1], and published a blog
> post
> > demonstrating usage of the package [2]. Special thanks to dmlc/nnvm team,
> > whose ONNX code was used as a reference for this implementation.
> >
> >
> >
> > What is ONNX?
> >
> > ONNX is an open source format to encode deep learning models. ONNX
> defines
> > a format to store neural network's computational graph, as well as a
> > storage format for operators used within a neural network graph. For more
> > details, check out onnx.ai [3].
> >
> >
> >
> > Why I think ONNX is important for MXNet?
> >
> > ONNX is an emerging standard, that holds a lot of potential for Deep
> > Learning practitioners. With ONNX, people can create and train a network
> > with framework A, and deploy it for inference with framework B. The blog
> > post we published demonstrates using a Super Res model trained with
> > PyTorch, and importing it into MXNet Symbolic API for inference. I
> strongly
> > believe that adopting ONNX early on adds value for deep learning
> > practitioners, and thus supporting it adds value for MXNet as well.
> >
> >
> >
> > As for next steps, I was thinking that porting the functionality and code
> > into MXNet is the logical next step.
> >
> > Would love to get the community's feedback and contributions!
> >
> >
> >
> > [1] https://github.com/onnx/onnx-mxnet
> >
> > [2] https://aws.amazon.com/blogs/ai/announcing-onnx-support-
> > for-apache-mxnet/
> >
> > [3] https://onnx.ai
> >
> >
>