RE: Time out for Travis CI
Hi Kellen, Thanks for your explanation. Do you have a time plan to solve the timeout issue? Rebasing can't work for my case. Or shall we run it silently to disallow it voting X for overall CI result? Because most developers are used to ignore the PRs with 'X'. Thanks, Zhennan -Original Message- From: kellen sunderland [mailto:kellen.sunderl...@gmail.com] Sent: Friday, September 28, 2018 10:38 PM To: dev@mxnet.incubator.apache.org Subject: Re: Time out for Travis CI Hey Zhennan, you're safe to ignore Travis failures for now. They're just informational. The reason you sometimes see quick builds and sometimes see slow builds is that we're making use of ccache in between builds. If your PR is similar to what's in master you should build very quickly, if not it's going to take a while and likely time out. If you see timeouts rebasing may speed things up. Unfortunately the timeouts are global and we're not able to increase them. I'm hoping that adding artifact caching will speed up future builds to the point that test runs and builds can be executed in under the global limit (which is ~50 minutes). -Kellen On Fri, Sep 28, 2018 at 4:05 PM Qin, Zhennan wrote: > Hi MXNet devs, > > I'm struggled with new Travis CI for a while, it always run time out > for this PR: > https://github.com/apache/incubator-mxnet/pull/12530 > > Most of the time, Jenkins CI can pass, while Travis can't be finished > within 50 minutes. For this PR, it shouldn't affect much on the build > time or unit test time. Also, I saw other PR has same problem, eg. > > > https://travis-ci.org/apache/incubator-mxnet/builds/433172088?utm_sour > ce=github_status_medium=notification > > https://travis-ci.org/apache/incubator-mxnet/builds/434404305?utm_sour > ce=github_status_medium=notification > > According to the time stamp from Travis, all passed PR are within > small code change, and can complete `make -j2` within 25s. But for > timeout case, 'make -j2' will need about 1600s. Does Travis do > incremental build for each test? Shall we increase time limit for > large PR? Can we add more time stamp for build and unites stage to help > understand what's going on there? > > Thanks in advance, > Zhennan >
Re: [Discuss] Next MXNet release
Sorry I meant to say next 'Regarding the *minor* release'. On Sat, Sep 29, 2018 at 5:27 AM kellen sunderland < kellen.sunderl...@gmail.com> wrote: > Thanks for transparently setting a rough timeline Steffen. I think this > will go a long way in helping the community plan their work, even if the > details change somewhat on the road to the release. > > Regarding the major release: I would propose we unify TensorRT with the > subgraph operator work. > > Regarding the patch release: There were a few minor stack/buffer > overflows exposed by ASAN that have been addressed. It's probably a good > idea to include them in a patch release, as they at best result in > non-deterministic behaviour. > > -Kellen > > > On Sat, Sep 29, 2018 at 1:39 AM Steffen Rochel > wrote: > >> I updated >> >> https://cwiki.apache.org/confluence/display/MXNET/Project+Proposals+for+next+MXNet+Release >> , >> removed the completed items from 1.3 release and would like to kick off >> discussion about the next release. Please suggest what you would like to >> see included in the next release together with link to design proposal >> (appropriately for the size and complexity of the proposal) or suggest >> changes. >> I suggest to target the next release for December 2018 to frame the >> discussion. >> Lets include review of >> https://cwiki.apache.org/confluence/display/MXNET/MXNet+Roadmap - time to >> update and discuss changes. >> >> From the 1.3 release we had discussion regarding >> https://github.com/apache/incubator-mxnet/issues/11849 and resolution in >> https://github.com/apache/incubator-mxnet/pull/12412 . >> Are you aware of critical issues and feedback from user which we should >> consider for a potential 1.3.1 patch release. Should we include PR 12412 >> in >> a potential patch release? >> >> Regards, >> Steffen >> >
Re: [Discuss] Next MXNet release
Thanks for transparently setting a rough timeline Steffen. I think this will go a long way in helping the community plan their work, even if the details change somewhat on the road to the release. Regarding the major release: I would propose we unify TensorRT with the subgraph operator work. Regarding the patch release: There were a few minor stack/buffer overflows exposed by ASAN that have been addressed. It's probably a good idea to include them in a patch release, as they at best result in non-deterministic behaviour. -Kellen On Sat, Sep 29, 2018 at 1:39 AM Steffen Rochel wrote: > I updated > > https://cwiki.apache.org/confluence/display/MXNET/Project+Proposals+for+next+MXNet+Release > , > removed the completed items from 1.3 release and would like to kick off > discussion about the next release. Please suggest what you would like to > see included in the next release together with link to design proposal > (appropriately for the size and complexity of the proposal) or suggest > changes. > I suggest to target the next release for December 2018 to frame the > discussion. > Lets include review of > https://cwiki.apache.org/confluence/display/MXNET/MXNet+Roadmap - time to > update and discuss changes. > > From the 1.3 release we had discussion regarding > https://github.com/apache/incubator-mxnet/issues/11849 and resolution in > https://github.com/apache/incubator-mxnet/pull/12412 . > Are you aware of critical issues and feedback from user which we should > consider for a potential 1.3.1 patch release. Should we include PR 12412 in > a potential patch release? > > Regards, > Steffen >
Re: Which merge option to use on the Import Julia binding PR?
Are we sure that this is due to lacking permissions and not because of some technical limitation? If we are certain, we can ask out mentors to create a ticket with Apache Infra to make that switch. -Marco Carin Meier schrieb am Sa., 29. Sep. 2018, 01:17: > I made a test regular merge commit into a copy of master. It seemed to go > fine. Here is a listing of what it will look like for everyone. > > https://github.com/apache/incubator-mxnet/commits/test-merge-julia-import > > Although, I would be happy to push the merge button. I think the most > important thing is to get the PR merged, so whatever way is the best to > make that happen, let's do it. > > So - Does the regular merge seem like a good option? > If so, what is the best way to make that happen? > > > On Fri, Sep 28, 2018 at 6:05 PM Chiyuan Zhang wrote: > > > Agreed with Pedro. Maybe the merge-commit option from the github > interface > > was disabled for a reason. But as Pedro said, maybe it is good to > > temporarily enable it for this PR and merge using that. > > > > > >- It should be technically easier than rebasing due to the > >git-subtree-import issue we are currently having > >- It also avoid stacking a huge commit history on *top* of current > >history > >- The downside is probably the history of the project is not linear > >anymore, but I think this is actually what we would like to have for > > this > >particular case, because the contents of the main repo and the julia > > branch > >actually does not overlap. So it makes sense to have two tails with > > their > >own history. > > > > Carin: I guess if someone with admin permission on the github could > > temporarily enable the merge-commit option, then pushing the button on > the > > web might simply work. > > > > Best, > > Chiyuan > > > > On Fri, Sep 28, 2018 at 2:53 PM Carin Meier > wrote: > > > > > Pedro - Maybe a merge commit is a better answer in this case. I > > originally > > > ruled it out since it wasn't an option in the github web interface, but > > > since this looks like it is going to have to be done outside it because > > of > > > the subtrees anyway, it might be a better fit. > > > > > > On Fri, Sep 28, 2018 at 5:07 PM Carin Meier > > wrote: > > > > > > > We are actually running into troubles with using the subtree and the > > > > rebase. Since it looks like this is not going to be a simple, "click > > the > > > > button" through the web page merge, I rather hand this task off to > > > someone > > > > with more context in making sure it gets in there correctly. > > > > > > > > Chiyuan or any others, would you be willing to take this over? > > > > > > > > Thanks, > > > > Carin > > > > > > > > On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy > > wrote: > > > > > > > >> Should we try to first being into a branch and then try merge that > > > >> branch? > > > >> > > > >> > On Sep 28, 2018, at 4:40 PM, Pedro Larroy < > > > pedro.larroy.li...@gmail.com> > > > >> wrote: > > > >> > > > > >> > I'm not familiar with the specifics of this contribution, as a > > general > > > >> > approach my understanding is that if the list of commits is big > and > > > you > > > >> > want to preserve history, usually merging is better so you keep > > > history > > > >> and > > > >> > causality, if you rebase all the commits on top of master you are > > > >> changing > > > >> > the history of these commits which can't be individually reverted > as > > > >> some > > > >> > have suggested before. Maybe is because I come from a mercurial > > > >> background, > > > >> > but my initial impression would be either to: > > > >> > 1. squash everything and rebase > > > >> > 2. or merge without rebasing or squashing. > > > >> > > > > >> > Pedro. > > > >> > > > > >> >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier < > carinme...@gmail.com> > > > >> wrote: > > > >> >> > > > >> >> Thanks everyone for the input. I'll try to summarize the feedback > > > from > > > >> the > > > >> >> responses: > > > >> >> > > > >> >> Using Squash-Merge is the project standard for very good reasons. > > > >> However, > > > >> >> in the case of this PR to bring in the Julia language from its > > > sibling > > > >> >> repo, we want to preserve all the individual commits of the many > > > >> >> contributors that have worked over multiple years to make this a > > > great > > > >> >> language binding. We will use Rebase-Merge for it. > > > >> >> > > > >> >> Chiyuan - thanks for the suggestion of using a tag. I think we > can > > > try > > > >> it > > > >> >> initially without it since there are other ways to browse the > > commit > > > >> >> history, like looking at the PRs. But, we can add the tag > > > >> retroactively if > > > >> >> people start having trouble. > > > >> >> > > > >> >> If there no objections, I will merge the PR using the above > method > > in > > > >> my > > > >> >> morning (EST). > > > >> >> > > > >> >> Thanks everyone! I'm looking forward to having the Julia > community > > >
Subscription to the ASF mxnet channel
Hi there, I am Javier, I was posting before on slack and I was advised to subscribe to that channel just that :) Ciao J.
[Discuss] Next MXNet release
I updated https://cwiki.apache.org/confluence/display/MXNET/Project+Proposals+for+next+MXNet+Release, removed the completed items from 1.3 release and would like to kick off discussion about the next release. Please suggest what you would like to see included in the next release together with link to design proposal (appropriately for the size and complexity of the proposal) or suggest changes. I suggest to target the next release for December 2018 to frame the discussion. Lets include review of https://cwiki.apache.org/confluence/display/MXNET/MXNet+Roadmap - time to update and discuss changes. >From the 1.3 release we had discussion regarding https://github.com/apache/incubator-mxnet/issues/11849 and resolution in https://github.com/apache/incubator-mxnet/pull/12412 . Are you aware of critical issues and feedback from user which we should consider for a potential 1.3.1 patch release. Should we include PR 12412 in a potential patch release? Regards, Steffen
Re: Which merge option to use on the Import Julia binding PR?
I made a test regular merge commit into a copy of master. It seemed to go fine. Here is a listing of what it will look like for everyone. https://github.com/apache/incubator-mxnet/commits/test-merge-julia-import Although, I would be happy to push the merge button. I think the most important thing is to get the PR merged, so whatever way is the best to make that happen, let's do it. So - Does the regular merge seem like a good option? If so, what is the best way to make that happen? On Fri, Sep 28, 2018 at 6:05 PM Chiyuan Zhang wrote: > Agreed with Pedro. Maybe the merge-commit option from the github interface > was disabled for a reason. But as Pedro said, maybe it is good to > temporarily enable it for this PR and merge using that. > > >- It should be technically easier than rebasing due to the >git-subtree-import issue we are currently having >- It also avoid stacking a huge commit history on *top* of current >history >- The downside is probably the history of the project is not linear >anymore, but I think this is actually what we would like to have for > this >particular case, because the contents of the main repo and the julia > branch >actually does not overlap. So it makes sense to have two tails with > their >own history. > > Carin: I guess if someone with admin permission on the github could > temporarily enable the merge-commit option, then pushing the button on the > web might simply work. > > Best, > Chiyuan > > On Fri, Sep 28, 2018 at 2:53 PM Carin Meier wrote: > > > Pedro - Maybe a merge commit is a better answer in this case. I > originally > > ruled it out since it wasn't an option in the github web interface, but > > since this looks like it is going to have to be done outside it because > of > > the subtrees anyway, it might be a better fit. > > > > On Fri, Sep 28, 2018 at 5:07 PM Carin Meier > wrote: > > > > > We are actually running into troubles with using the subtree and the > > > rebase. Since it looks like this is not going to be a simple, "click > the > > > button" through the web page merge, I rather hand this task off to > > someone > > > with more context in making sure it gets in there correctly. > > > > > > Chiyuan or any others, would you be willing to take this over? > > > > > > Thanks, > > > Carin > > > > > > On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy > wrote: > > > > > >> Should we try to first being into a branch and then try merge that > > >> branch? > > >> > > >> > On Sep 28, 2018, at 4:40 PM, Pedro Larroy < > > pedro.larroy.li...@gmail.com> > > >> wrote: > > >> > > > >> > I'm not familiar with the specifics of this contribution, as a > general > > >> > approach my understanding is that if the list of commits is big and > > you > > >> > want to preserve history, usually merging is better so you keep > > history > > >> and > > >> > causality, if you rebase all the commits on top of master you are > > >> changing > > >> > the history of these commits which can't be individually reverted as > > >> some > > >> > have suggested before. Maybe is because I come from a mercurial > > >> background, > > >> > but my initial impression would be either to: > > >> > 1. squash everything and rebase > > >> > 2. or merge without rebasing or squashing. > > >> > > > >> > Pedro. > > >> > > > >> >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier > > >> wrote: > > >> >> > > >> >> Thanks everyone for the input. I'll try to summarize the feedback > > from > > >> the > > >> >> responses: > > >> >> > > >> >> Using Squash-Merge is the project standard for very good reasons. > > >> However, > > >> >> in the case of this PR to bring in the Julia language from its > > sibling > > >> >> repo, we want to preserve all the individual commits of the many > > >> >> contributors that have worked over multiple years to make this a > > great > > >> >> language binding. We will use Rebase-Merge for it. > > >> >> > > >> >> Chiyuan - thanks for the suggestion of using a tag. I think we can > > try > > >> it > > >> >> initially without it since there are other ways to browse the > commit > > >> >> history, like looking at the PRs. But, we can add the tag > > >> retroactively if > > >> >> people start having trouble. > > >> >> > > >> >> If there no objections, I will merge the PR using the above method > in > > >> my > > >> >> morning (EST). > > >> >> > > >> >> Thanks everyone! I'm looking forward to having the Julia community > > >> join the > > >> >> main repo and increasing our collaboration with them. > > >> >> > > >> >> Best, > > >> >> Carin > > >> >> > > >> >>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang > > >> wrote: > > >> >>> > > >> >>> +1 for rebase and merge. As a workaround for the aforementioned > > issue, > > >> >>> maybe we can create a tag for the commit before the merge, so that > > in > > >> >> case > > >> >>> people want to browse the recent main-repo commits by skipping > this > > >> big > > >> >>> chunk of rebased commits, there is a pointer to
Re: Which merge option to use on the Import Julia binding PR?
Agreed with Pedro. Maybe the merge-commit option from the github interface was disabled for a reason. But as Pedro said, maybe it is good to temporarily enable it for this PR and merge using that. - It should be technically easier than rebasing due to the git-subtree-import issue we are currently having - It also avoid stacking a huge commit history on *top* of current history - The downside is probably the history of the project is not linear anymore, but I think this is actually what we would like to have for this particular case, because the contents of the main repo and the julia branch actually does not overlap. So it makes sense to have two tails with their own history. Carin: I guess if someone with admin permission on the github could temporarily enable the merge-commit option, then pushing the button on the web might simply work. Best, Chiyuan On Fri, Sep 28, 2018 at 2:53 PM Carin Meier wrote: > Pedro - Maybe a merge commit is a better answer in this case. I originally > ruled it out since it wasn't an option in the github web interface, but > since this looks like it is going to have to be done outside it because of > the subtrees anyway, it might be a better fit. > > On Fri, Sep 28, 2018 at 5:07 PM Carin Meier wrote: > > > We are actually running into troubles with using the subtree and the > > rebase. Since it looks like this is not going to be a simple, "click the > > button" through the web page merge, I rather hand this task off to > someone > > with more context in making sure it gets in there correctly. > > > > Chiyuan or any others, would you be willing to take this over? > > > > Thanks, > > Carin > > > > On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy wrote: > > > >> Should we try to first being into a branch and then try merge that > >> branch? > >> > >> > On Sep 28, 2018, at 4:40 PM, Pedro Larroy < > pedro.larroy.li...@gmail.com> > >> wrote: > >> > > >> > I'm not familiar with the specifics of this contribution, as a general > >> > approach my understanding is that if the list of commits is big and > you > >> > want to preserve history, usually merging is better so you keep > history > >> and > >> > causality, if you rebase all the commits on top of master you are > >> changing > >> > the history of these commits which can't be individually reverted as > >> some > >> > have suggested before. Maybe is because I come from a mercurial > >> background, > >> > but my initial impression would be either to: > >> > 1. squash everything and rebase > >> > 2. or merge without rebasing or squashing. > >> > > >> > Pedro. > >> > > >> >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier > >> wrote: > >> >> > >> >> Thanks everyone for the input. I'll try to summarize the feedback > from > >> the > >> >> responses: > >> >> > >> >> Using Squash-Merge is the project standard for very good reasons. > >> However, > >> >> in the case of this PR to bring in the Julia language from its > sibling > >> >> repo, we want to preserve all the individual commits of the many > >> >> contributors that have worked over multiple years to make this a > great > >> >> language binding. We will use Rebase-Merge for it. > >> >> > >> >> Chiyuan - thanks for the suggestion of using a tag. I think we can > try > >> it > >> >> initially without it since there are other ways to browse the commit > >> >> history, like looking at the PRs. But, we can add the tag > >> retroactively if > >> >> people start having trouble. > >> >> > >> >> If there no objections, I will merge the PR using the above method in > >> my > >> >> morning (EST). > >> >> > >> >> Thanks everyone! I'm looking forward to having the Julia community > >> join the > >> >> main repo and increasing our collaboration with them. > >> >> > >> >> Best, > >> >> Carin > >> >> > >> >>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang > >> wrote: > >> >>> > >> >>> +1 for rebase and merge. As a workaround for the aforementioned > issue, > >> >>> maybe we can create a tag for the commit before the merge, so that > in > >> >> case > >> >>> people want to browse the recent main-repo commits by skipping this > >> big > >> >>> chunk of rebased commits, there is a pointer to take his or her hand > >> on. > >> >>> > >> >>> Best, > >> >>> Chiyuan > >> >>> > >> On Thu, Sep 27, 2018 at 7:34 AM Jason Dai > >> wrote: > >> > >> +1 to rebase and merge to preserve and track the contributions. > >> > >> Thanks, > >> -Jason > >> > >> On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham < > >> >>> aaron.s.mark...@gmail.com> > >> wrote: > >> > >> > +1 to rebase and merge to retain the efforts of all of the > >> >>> contributors. > >> If > >> > there's some git maintenance that can trim it down from 700+ > commits > >> >>> then > >> > maybe that's a compromise. > >> > > >> >> On Wed, Sep 26, 2018, 21:23 Naveen Swamy > >> wrote: > >> >> > >> >> this PR comes from more than 1 individual, if we
Re: Which merge option to use on the Import Julia binding PR?
Pedro - Maybe a merge commit is a better answer in this case. I originally ruled it out since it wasn't an option in the github web interface, but since this looks like it is going to have to be done outside it because of the subtrees anyway, it might be a better fit. On Fri, Sep 28, 2018 at 5:07 PM Carin Meier wrote: > We are actually running into troubles with using the subtree and the > rebase. Since it looks like this is not going to be a simple, "click the > button" through the web page merge, I rather hand this task off to someone > with more context in making sure it gets in there correctly. > > Chiyuan or any others, would you be willing to take this over? > > Thanks, > Carin > > On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy wrote: > >> Should we try to first being into a branch and then try merge that >> branch? >> >> > On Sep 28, 2018, at 4:40 PM, Pedro Larroy >> wrote: >> > >> > I'm not familiar with the specifics of this contribution, as a general >> > approach my understanding is that if the list of commits is big and you >> > want to preserve history, usually merging is better so you keep history >> and >> > causality, if you rebase all the commits on top of master you are >> changing >> > the history of these commits which can't be individually reverted as >> some >> > have suggested before. Maybe is because I come from a mercurial >> background, >> > but my initial impression would be either to: >> > 1. squash everything and rebase >> > 2. or merge without rebasing or squashing. >> > >> > Pedro. >> > >> >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier >> wrote: >> >> >> >> Thanks everyone for the input. I'll try to summarize the feedback from >> the >> >> responses: >> >> >> >> Using Squash-Merge is the project standard for very good reasons. >> However, >> >> in the case of this PR to bring in the Julia language from its sibling >> >> repo, we want to preserve all the individual commits of the many >> >> contributors that have worked over multiple years to make this a great >> >> language binding. We will use Rebase-Merge for it. >> >> >> >> Chiyuan - thanks for the suggestion of using a tag. I think we can try >> it >> >> initially without it since there are other ways to browse the commit >> >> history, like looking at the PRs. But, we can add the tag >> retroactively if >> >> people start having trouble. >> >> >> >> If there no objections, I will merge the PR using the above method in >> my >> >> morning (EST). >> >> >> >> Thanks everyone! I'm looking forward to having the Julia community >> join the >> >> main repo and increasing our collaboration with them. >> >> >> >> Best, >> >> Carin >> >> >> >>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang >> wrote: >> >>> >> >>> +1 for rebase and merge. As a workaround for the aforementioned issue, >> >>> maybe we can create a tag for the commit before the merge, so that in >> >> case >> >>> people want to browse the recent main-repo commits by skipping this >> big >> >>> chunk of rebased commits, there is a pointer to take his or her hand >> on. >> >>> >> >>> Best, >> >>> Chiyuan >> >>> >> On Thu, Sep 27, 2018 at 7:34 AM Jason Dai >> wrote: >> >> +1 to rebase and merge to preserve and track the contributions. >> >> Thanks, >> -Jason >> >> On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham < >> >>> aaron.s.mark...@gmail.com> >> wrote: >> >> > +1 to rebase and merge to retain the efforts of all of the >> >>> contributors. >> If >> > there's some git maintenance that can trim it down from 700+ commits >> >>> then >> > maybe that's a compromise. >> > >> >> On Wed, Sep 26, 2018, 21:23 Naveen Swamy >> wrote: >> >> >> >> this PR comes from more than 1 individual, if we squash merge we'll >> >>> not >> > be >> >> able to attribute the contribution of those individuals. >> >> >> >> +1 to rebase merge to preserve history >> >> >> >> On Thu, Sep 27, 2018 at 12:04 AM, Tianqi Chen < >> tqc...@cs.washington.edu> >> >> wrote: >> >> >> >>> One of the main reason for a rebase merge is that it preserves >> >> the >> > commit >> >>> history of the MXNet.jl package contributors, and given that the >> > project >> >>> has been evolved since 2015 and has always been a high-quality >> language >> >>> module for MXNet. >> >>> >> >>> I think we should take an exception here to preserve the commit >> history >> >> of >> >>> each individual contributors to the Julia binding and welcome >> >> them >> >>> to >> > the >> >>> community. >> >>> >> >>> Tianqi >> >>> >> >>> On Wed, Sep 26, 2018 at 8:55 PM Tianqi Chen < >> tqc...@cs.washington.edu> >> >>> wrote: >> >>> >> In this particular case, I would suggest rebase and merge. >> >> The main reasoning is that the commit log of the Julia binding >> >> is >> not >> simple WIP
Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project
ok then, my vote is still -1, however, because it’s just adding needless
friction for developers imho.
On Fri, Sep 28, 2018 at 7:42 AM kellen sunderland <
kellen.sunderl...@gmail.com> wrote:
> "Range loops aren’t always the most performant way" Do you have an example
> where there's a perf difference?
>
> "In addition, sometimes you want the index. Or maybe you want to iterate
> backwards, or not start from the first, etc. Maybe you want the iterator
> because you remove it from the list at the bottom of the loop Seems
> like a rule for the sake of having a rule."
>
> I should have been more clear about this point. If you're using the index
> in the loop, doing reverse iteration, or not iterating from start-to-end
> this inspection is smart enough to realize it and will not suggest
> optimizing that type of loop. The loops that would be changes are _only_
> the loops which are detected as equivalent to range-loops. Examples can be
> found here:
> https://clang.llvm.org/extra/clang-tidy/checks/modernize-loop-convert.html
> or you can look at what's been changed in the ref PR. I've initially set
> our confidence level at 'reasonable' but we could also set to 'safe' which
> would further reduce the number of loops the check would apply to.
>
> -Kellen
>
> On Fri, Sep 28, 2018 at 3:54 PM Chris Olivier
> wrote:
>
> > -1
> >
> > Range loops aren’t always the most performant way. In addition, sometimes
> > you want the index. Or maybe you want to iterate backwards, or not start
> > from the first, etc. Maybe you want the iterator because you remove it
> from
> > the list at the bottom of the loop Seems like a rule for the sake of
> > having a rule.
> >
> > On Fri, Sep 28, 2018 at 2:12 AM kellen sunderland <
> > kellen.sunderl...@gmail.com> wrote:
> >
> > > Hello MXNet devs,
> > >
> > > I'd like to discuss uniformly adopting C++11 range loops in the MXNet
> > > project. The benefits I see are:
> > >
> > > * Improved C++ readability (examples below).
> > > * Consistency with other languages. The range-loops are quite similar
> > to
> > > loops almost all other programming languages. Given we're a project
> that
> > > supports many languages this language consistency could be positive for
> > our
> > > community.
> > > * Consistency within the same project. Currently different authors
> have
> > > different loops styles which hurts codebase readability.
> > > * Best available performance. There are often multiple ways to write
> > > loops in C++ with subtle differences in performance and memory usage
> > > between loop methods. Using range-loops ensures we get the best
> possible
> > > perf using an intuitive loop pattern.
> > > * Slightly lower chance for bugs / OOB accesses when dealing with
> > indexing
> > > in an array for example.
> > >
> > > If we decide to enable this uniformly throughout the project we can
> > enable
> > > this policy with a simple clang-tidy configuration change. There would
> > be
> > > no need for reviewers to have to manually provide feedback when someone
> > > uses an older C++ loops style.
> > >
> > > -Kellen
> > >
> > > Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/
> > > Previous clang-tidy discussion on the list:
> > >
> > >
> >
> https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E
> > >
> > > -
> > > Examples:
> > > for (auto axis_iter = param.axis.begin() ; axis_iter!=
> param.axis.end();
> > > ++axis_iter) {
> > > CHECK_LT(*axis_iter, static_cast(ishape.ndim()));
> > > stride_[reverse_index] = ishape[*axis_iter];
> > > ...
> > > -->
> > > for (int axis : param.axis) {
> > > CHECK_LT(axis, static_cast(ishape.ndim()));
> > > stride_[reverse_index] = ishape[axis];
> > > ...
> > > --
> > > for (size_t i = 0; i < in_array.size(); i++) {
> > > auto = in_array[i];
> > > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > > }
> > > -->
> > > for (auto & nd : in_array) {
> > > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > > }
> > >
> >
>
Re: Which merge option to use on the Import Julia binding PR?
We are actually running into troubles with using the subtree and the rebase. Since it looks like this is not going to be a simple, "click the button" through the web page merge, I rather hand this task off to someone with more context in making sure it gets in there correctly. Chiyuan or any others, would you be willing to take this over? Thanks, Carin On Fri, Sep 28, 2018 at 5:00 PM Naveen Swamy wrote: > Should we try to first being into a branch and then try merge that branch? > > > On Sep 28, 2018, at 4:40 PM, Pedro Larroy > wrote: > > > > I'm not familiar with the specifics of this contribution, as a general > > approach my understanding is that if the list of commits is big and you > > want to preserve history, usually merging is better so you keep history > and > > causality, if you rebase all the commits on top of master you are > changing > > the history of these commits which can't be individually reverted as some > > have suggested before. Maybe is because I come from a mercurial > background, > > but my initial impression would be either to: > > 1. squash everything and rebase > > 2. or merge without rebasing or squashing. > > > > Pedro. > > > >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier > wrote: > >> > >> Thanks everyone for the input. I'll try to summarize the feedback from > the > >> responses: > >> > >> Using Squash-Merge is the project standard for very good reasons. > However, > >> in the case of this PR to bring in the Julia language from its sibling > >> repo, we want to preserve all the individual commits of the many > >> contributors that have worked over multiple years to make this a great > >> language binding. We will use Rebase-Merge for it. > >> > >> Chiyuan - thanks for the suggestion of using a tag. I think we can try > it > >> initially without it since there are other ways to browse the commit > >> history, like looking at the PRs. But, we can add the tag retroactively > if > >> people start having trouble. > >> > >> If there no objections, I will merge the PR using the above method in my > >> morning (EST). > >> > >> Thanks everyone! I'm looking forward to having the Julia community join > the > >> main repo and increasing our collaboration with them. > >> > >> Best, > >> Carin > >> > >>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang > wrote: > >>> > >>> +1 for rebase and merge. As a workaround for the aforementioned issue, > >>> maybe we can create a tag for the commit before the merge, so that in > >> case > >>> people want to browse the recent main-repo commits by skipping this big > >>> chunk of rebased commits, there is a pointer to take his or her hand > on. > >>> > >>> Best, > >>> Chiyuan > >>> > On Thu, Sep 27, 2018 at 7:34 AM Jason Dai > wrote: > > +1 to rebase and merge to preserve and track the contributions. > > Thanks, > -Jason > > On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham < > >>> aaron.s.mark...@gmail.com> > wrote: > > > +1 to rebase and merge to retain the efforts of all of the > >>> contributors. > If > > there's some git maintenance that can trim it down from 700+ commits > >>> then > > maybe that's a compromise. > > > >> On Wed, Sep 26, 2018, 21:23 Naveen Swamy > wrote: > >> > >> this PR comes from more than 1 individual, if we squash merge we'll > >>> not > > be > >> able to attribute the contribution of those individuals. > >> > >> +1 to rebase merge to preserve history > >> > >> On Thu, Sep 27, 2018 at 12:04 AM, Tianqi Chen < > tqc...@cs.washington.edu> > >> wrote: > >> > >>> One of the main reason for a rebase merge is that it preserves > >> the > > commit > >>> history of the MXNet.jl package contributors, and given that the > > project > >>> has been evolved since 2015 and has always been a high-quality > language > >>> module for MXNet. > >>> > >>> I think we should take an exception here to preserve the commit > history > >> of > >>> each individual contributors to the Julia binding and welcome > >> them > >>> to > > the > >>> community. > >>> > >>> Tianqi > >>> > >>> On Wed, Sep 26, 2018 at 8:55 PM Tianqi Chen < > tqc...@cs.washington.edu> > >>> wrote: > >>> > In this particular case, I would suggest rebase and merge. > > The main reasoning is that the commit log of the Julia binding > >> is > not > simple WIP commits, every commit there has been done through > > testcases > >>> and > it is important for us to respect the developer of the effort. > >> It > is > >> also > good to trace back the history of the commits more easily. > > Tianqi > > > Tianqi > > On Wed, Sep 26, 2018 at 5:34 PM Carin Meier < > >>> carinme...@gmail.com> > >>> wrote: > > > Chiyuan, > > >
Re: Which merge option to use on the Import Julia binding PR?
Should we try to first being into a branch and then try merge that branch? > On Sep 28, 2018, at 4:40 PM, Pedro Larroy > wrote: > > I'm not familiar with the specifics of this contribution, as a general > approach my understanding is that if the list of commits is big and you > want to preserve history, usually merging is better so you keep history and > causality, if you rebase all the commits on top of master you are changing > the history of these commits which can't be individually reverted as some > have suggested before. Maybe is because I come from a mercurial background, > but my initial impression would be either to: > 1. squash everything and rebase > 2. or merge without rebasing or squashing. > > Pedro. > >> On Thu, Sep 27, 2018 at 3:10 PM Carin Meier wrote: >> >> Thanks everyone for the input. I'll try to summarize the feedback from the >> responses: >> >> Using Squash-Merge is the project standard for very good reasons. However, >> in the case of this PR to bring in the Julia language from its sibling >> repo, we want to preserve all the individual commits of the many >> contributors that have worked over multiple years to make this a great >> language binding. We will use Rebase-Merge for it. >> >> Chiyuan - thanks for the suggestion of using a tag. I think we can try it >> initially without it since there are other ways to browse the commit >> history, like looking at the PRs. But, we can add the tag retroactively if >> people start having trouble. >> >> If there no objections, I will merge the PR using the above method in my >> morning (EST). >> >> Thanks everyone! I'm looking forward to having the Julia community join the >> main repo and increasing our collaboration with them. >> >> Best, >> Carin >> >>> On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang wrote: >>> >>> +1 for rebase and merge. As a workaround for the aforementioned issue, >>> maybe we can create a tag for the commit before the merge, so that in >> case >>> people want to browse the recent main-repo commits by skipping this big >>> chunk of rebased commits, there is a pointer to take his or her hand on. >>> >>> Best, >>> Chiyuan >>> On Thu, Sep 27, 2018 at 7:34 AM Jason Dai wrote: +1 to rebase and merge to preserve and track the contributions. Thanks, -Jason On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham < >>> aaron.s.mark...@gmail.com> wrote: > +1 to rebase and merge to retain the efforts of all of the >>> contributors. If > there's some git maintenance that can trim it down from 700+ commits >>> then > maybe that's a compromise. > >> On Wed, Sep 26, 2018, 21:23 Naveen Swamy wrote: >> >> this PR comes from more than 1 individual, if we squash merge we'll >>> not > be >> able to attribute the contribution of those individuals. >> >> +1 to rebase merge to preserve history >> >> On Thu, Sep 27, 2018 at 12:04 AM, Tianqi Chen < tqc...@cs.washington.edu> >> wrote: >> >>> One of the main reason for a rebase merge is that it preserves >> the > commit >>> history of the MXNet.jl package contributors, and given that the > project >>> has been evolved since 2015 and has always been a high-quality language >>> module for MXNet. >>> >>> I think we should take an exception here to preserve the commit history >> of >>> each individual contributors to the Julia binding and welcome >> them >>> to > the >>> community. >>> >>> Tianqi >>> >>> On Wed, Sep 26, 2018 at 8:55 PM Tianqi Chen < tqc...@cs.washington.edu> >>> wrote: >>> In this particular case, I would suggest rebase and merge. The main reasoning is that the commit log of the Julia binding >> is not simple WIP commits, every commit there has been done through > testcases >>> and it is important for us to respect the developer of the effort. >> It is >> also good to trace back the history of the commits more easily. Tianqi Tianqi On Wed, Sep 26, 2018 at 5:34 PM Carin Meier < >>> carinme...@gmail.com> >>> wrote: > Chiyuan, > > Thanks for the prompt to find some clarity of the pros and >> cons >>> of >>> each. I > think that will help drive us to the right decision. I think >>> some of >>> those > reasons are the ones you listed. I will take a stab below at > outlining > what > I see. Feel free to chime in if I missed any. > > *Squash and Merge* > *Pros* - It is the project standard > - It will provide one commit for the feature and >>> lessen > the >>> need > for 700+ commits rebased on top of master. > - It is easier for a user to do git log to browse
Re: Maturity Model and Graduation
So Isabel, are you saying that if we publish a clearer TODO list or contributions needed material we might get more contribution there? One thing that I like from other projects is to make a list of low-hanging fruit issues or easy contributions that newcomers can pick to get familiar with the project, especially in projects like MXNet in which some contributions might require significant ramp up time, technical and mathematical skills or domain knowledge. Pedro. On Fri, Sep 28, 2018 at 3:06 AM Isabel Drost-Fromm wrote: > > > On 28/09/18 11:27, kellen sunderland wrote: > > I'd love to see some more > > sustained contribution from other open source communities to help us out > in > > this area > > That's not exactly the model I have seen to work. What I have seen works > really well at other projects is pulling users in as committers in a > scratch your own itch kind of way. For that to work you need to make it > clear what contributions you need, you need to make time to coach people > to become developers, you need to make your users accustomed to the way > you work as early as possible. It also helps to ask users for > contributions and offer mentoring help from your side along the way. > > I know that this is tedious work that needs a lot of motivating people, > mentoring people, explaining to people, however it makes for a > sustainable community of people that do the work out of self interest. > > > http://blog.isabel-drost.de/posts/open-development-and-inner-source-for-fun-and-profit.html > > > Isabel >
Re: Which merge option to use on the Import Julia binding PR?
I'm not familiar with the specifics of this contribution, as a general approach my understanding is that if the list of commits is big and you want to preserve history, usually merging is better so you keep history and causality, if you rebase all the commits on top of master you are changing the history of these commits which can't be individually reverted as some have suggested before. Maybe is because I come from a mercurial background, but my initial impression would be either to: 1. squash everything and rebase 2. or merge without rebasing or squashing. Pedro. On Thu, Sep 27, 2018 at 3:10 PM Carin Meier wrote: > Thanks everyone for the input. I'll try to summarize the feedback from the > responses: > > Using Squash-Merge is the project standard for very good reasons. However, > in the case of this PR to bring in the Julia language from its sibling > repo, we want to preserve all the individual commits of the many > contributors that have worked over multiple years to make this a great > language binding. We will use Rebase-Merge for it. > > Chiyuan - thanks for the suggestion of using a tag. I think we can try it > initially without it since there are other ways to browse the commit > history, like looking at the PRs. But, we can add the tag retroactively if > people start having trouble. > > If there no objections, I will merge the PR using the above method in my > morning (EST). > > Thanks everyone! I'm looking forward to having the Julia community join the > main repo and increasing our collaboration with them. > > Best, > Carin > > On Thu, Sep 27, 2018 at 1:37 PM Chiyuan Zhang wrote: > > > +1 for rebase and merge. As a workaround for the aforementioned issue, > > maybe we can create a tag for the commit before the merge, so that in > case > > people want to browse the recent main-repo commits by skipping this big > > chunk of rebased commits, there is a pointer to take his or her hand on. > > > > Best, > > Chiyuan > > > > On Thu, Sep 27, 2018 at 7:34 AM Jason Dai wrote: > > > > > +1 to rebase and merge to preserve and track the contributions. > > > > > > Thanks, > > > -Jason > > > > > > On Thu, Sep 27, 2018 at 12:27 PM Aaron Markham < > > aaron.s.mark...@gmail.com> > > > wrote: > > > > > > > +1 to rebase and merge to retain the efforts of all of the > > contributors. > > > If > > > > there's some git maintenance that can trim it down from 700+ commits > > then > > > > maybe that's a compromise. > > > > > > > > On Wed, Sep 26, 2018, 21:23 Naveen Swamy wrote: > > > > > > > > > this PR comes from more than 1 individual, if we squash merge we'll > > not > > > > be > > > > > able to attribute the contribution of those individuals. > > > > > > > > > > +1 to rebase merge to preserve history > > > > > > > > > > On Thu, Sep 27, 2018 at 12:04 AM, Tianqi Chen < > > > tqc...@cs.washington.edu> > > > > > wrote: > > > > > > > > > > > One of the main reason for a rebase merge is that it preserves > the > > > > commit > > > > > > history of the MXNet.jl package contributors, and given that the > > > > project > > > > > > has been evolved since 2015 and has always been a high-quality > > > language > > > > > > module for MXNet. > > > > > > > > > > > > I think we should take an exception here to preserve the commit > > > history > > > > > of > > > > > > each individual contributors to the Julia binding and welcome > them > > to > > > > the > > > > > > community. > > > > > > > > > > > > Tianqi > > > > > > > > > > > > On Wed, Sep 26, 2018 at 8:55 PM Tianqi Chen < > > > tqc...@cs.washington.edu> > > > > > > wrote: > > > > > > > > > > > > > In this particular case, I would suggest rebase and merge. > > > > > > > > > > > > > > The main reasoning is that the commit log of the Julia binding > is > > > not > > > > > > > simple WIP commits, every commit there has been done through > > > > testcases > > > > > > and > > > > > > > it is important for us to respect the developer of the effort. > It > > > is > > > > > also > > > > > > > good to trace back the history of the commits more easily. > > > > > > > > > > > > > > Tianqi > > > > > > > > > > > > > > > > > > > > > Tianqi > > > > > > > > > > > > > > On Wed, Sep 26, 2018 at 5:34 PM Carin Meier < > > carinme...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > >> Chiyuan, > > > > > > >> > > > > > > >> Thanks for the prompt to find some clarity of the pros and > cons > > of > > > > > > each. I > > > > > > >> think that will help drive us to the right decision. I think > > some > > > of > > > > > > those > > > > > > >> reasons are the ones you listed. I will take a stab below at > > > > outlining > > > > > > >> what > > > > > > >> I see. Feel free to chime in if I missed any. > > > > > > >> > > > > > > >> *Squash and Merge* > > > > > > >> *Pros* - It is the project standard > > > > > > >> - It will provide one commit for the feature and > > lessen > > > > the > > > > > > need > > > > > > >> for 700+ commits rebased on top of master. > > > > > > >>
Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project
+1
Using range-based for-loop whenever possible improves code readability and
makes code less prone to human error.
I did some preliminary research on Google and did not find any complaint
about its performance drawback. Here is one piece from StackOverflow for
reference:
https://stackoverflow.com/questions/10821756/is-the-ranged-based-for-loop-beneficial-to-performance
Lin
On Fri, Sep 28, 2018 at 7:42 AM kellen sunderland <
kellen.sunderl...@gmail.com> wrote:
> "Range loops aren’t always the most performant way" Do you have an example
> where there's a perf difference?
>
> "In addition, sometimes you want the index. Or maybe you want to iterate
> backwards, or not start from the first, etc. Maybe you want the iterator
> because you remove it from the list at the bottom of the loop Seems
> like a rule for the sake of having a rule."
>
> I should have been more clear about this point. If you're using the index
> in the loop, doing reverse iteration, or not iterating from start-to-end
> this inspection is smart enough to realize it and will not suggest
> optimizing that type of loop. The loops that would be changes are _only_
> the loops which are detected as equivalent to range-loops. Examples can be
> found here:
> https://clang.llvm.org/extra/clang-tidy/checks/modernize-loop-convert.html
> or you can look at what's been changed in the ref PR. I've initially set
> our confidence level at 'reasonable' but we could also set to 'safe' which
> would further reduce the number of loops the check would apply to.
>
> -Kellen
>
> On Fri, Sep 28, 2018 at 3:54 PM Chris Olivier
> wrote:
>
> > -1
> >
> > Range loops aren’t always the most performant way. In addition, sometimes
> > you want the index. Or maybe you want to iterate backwards, or not start
> > from the first, etc. Maybe you want the iterator because you remove it
> from
> > the list at the bottom of the loop Seems like a rule for the sake of
> > having a rule.
> >
> > On Fri, Sep 28, 2018 at 2:12 AM kellen sunderland <
> > kellen.sunderl...@gmail.com> wrote:
> >
> > > Hello MXNet devs,
> > >
> > > I'd like to discuss uniformly adopting C++11 range loops in the MXNet
> > > project. The benefits I see are:
> > >
> > > * Improved C++ readability (examples below).
> > > * Consistency with other languages. The range-loops are quite similar
> > to
> > > loops almost all other programming languages. Given we're a project
> that
> > > supports many languages this language consistency could be positive for
> > our
> > > community.
> > > * Consistency within the same project. Currently different authors
> have
> > > different loops styles which hurts codebase readability.
> > > * Best available performance. There are often multiple ways to write
> > > loops in C++ with subtle differences in performance and memory usage
> > > between loop methods. Using range-loops ensures we get the best
> possible
> > > perf using an intuitive loop pattern.
> > > * Slightly lower chance for bugs / OOB accesses when dealing with
> > indexing
> > > in an array for example.
> > >
> > > If we decide to enable this uniformly throughout the project we can
> > enable
> > > this policy with a simple clang-tidy configuration change. There would
> > be
> > > no need for reviewers to have to manually provide feedback when someone
> > > uses an older C++ loops style.
> > >
> > > -Kellen
> > >
> > > Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/
> > > Previous clang-tidy discussion on the list:
> > >
> > >
> >
> https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E
> > >
> > > -
> > > Examples:
> > > for (auto axis_iter = param.axis.begin() ; axis_iter!=
> param.axis.end();
> > > ++axis_iter) {
> > > CHECK_LT(*axis_iter, static_cast(ishape.ndim()));
> > > stride_[reverse_index] = ishape[*axis_iter];
> > > ...
> > > -->
> > > for (int axis : param.axis) {
> > > CHECK_LT(axis, static_cast(ishape.ndim()));
> > > stride_[reverse_index] = ishape[axis];
> > > ...
> > > --
> > > for (size_t i = 0; i < in_array.size(); i++) {
> > > auto = in_array[i];
> > > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > > }
> > > -->
> > > for (auto & nd : in_array) {
> > > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > > }
> > >
> >
>
Re: [LAZY VOTE] Consolidating developer guide in one place (cwiki preferred)
Hi Aaron, Thanks a lot for effort. This consolidation will make it more convenient for developers to find development resource and help to attract more contributors. I have also created a story to make it easy for developers to navigate from mxnet.io: https://issues.apache.org/jira/browse/MXNET-1002 Thanks! Lin On Wed, Sep 26, 2018 at 10:24 AM Aaron Markham wrote: > I think the latest feedback has been great. It seems to be mostly user > level issues though. Installation and usage primarily, with a sprinkle of > *if that stuff was better then I might be able to contribute*. > > I've (with a few other contributors) tackled some of the very direct bits > of feedback for the website by incremental improvement of the install > pages, Gluon info, and UX for the API docs. > > I've started additional planning for updates by adding an epic with > specific stories and tasks to Jira for the documentation pipeline (the > backend part of the website build): > https://issues.apache.org/jira/browse/MXNET-957 > > I've also added one that is more specific to the website's content: > https://issues.apache.org/jira/browse/MXNET-986 > This is where I've captured only two tasks related to transitioning content > related to "contributing to MXNet" over to the wiki. Any pointers on which > content to move would help. These could be added as tasks too. > > I welcome any suggestions, additions, and contributions to either of these > epics. > > Cheers, > Aaron > > On Wed, Sep 26, 2018, 00:02 Lin Yuan wrote: > > > Hi Aaron, > > > > Do we have a resolution for this proposal yet? Recently, there have been > > many asks for a better documentation for MXNet developers. I think it's a > > good time that we consolidate the developer documentation in a central > > place. Any thoughts or plan? > > > > Many Thanks, > > > > Lin > > > > On Tue, Sep 4, 2018 at 1:55 PM Lin Yuan wrote: > > > > > +1 > > > > > > On Tue, Sep 4, 2018 at 1:46 PM Aaron Markham < > aaron.s.mark...@gmail.com> > > > wrote: > > > > > >> I'd like to call for a lazy vote on this before proceeding. Already > had > > >> some +1s but let's be sure. > > >> > > >> The vote is to move developer guide info to cwiki. User guides would > > >> remain > > >> on the website. > > >> > > >> On Tue, Aug 21, 2018 at 12:53 PM sandeep krishnamurthy < > > >> sandeep.krishn...@gmail.com> wrote: > > >> > > >> > +1 > > >> > Thanks Lin and Aaron. I agree website to cover all user facing > > >> > documentation and a separate consolidated and organized developer > > >> focussed > > >> > docs in one place (cwiki). > > >> > > > >> > > > >> > Note: Permissions on cwiki is currently not well managed with many > > >> people > > >> > having full admin rights to edit/create/delete pages. Should be fine > > for > > >> > now, but, when we start accumulating many documents and resources, > we > > >> > should probably revisit on Delete permissions. > > >> > > > >> > > > >> > On Tue, Aug 21, 2018 at 11:57 AM Lin Yuan > > wrote: > > >> > > > >> > > Hi Aaron, > > >> > > > > >> > > Thanks for your answer. I think it's a very worthwhile effort to > > move > > >> all > > >> > > the developer related content from mxet.io website to a dedicated > > >> > > developer > > >> > > site. Would you like to initiate this effort? > > >> > > > > >> > > Best, > > >> > > > > >> > > Lin > > >> > > > > >> > > On Wed, Aug 15, 2018 at 3:47 PM Haibin Lin < > > haibin.lin@gmail.com> > > >> > > wrote: > > >> > > > > >> > > > +1 > > >> > > > > > >> > > > On Wed, Aug 15, 2018 at 1:10 PM, Aaron Markham < > > >> > > aaron.s.mark...@gmail.com> > > >> > > > wrote: > > >> > > > > > >> > > > > Hi Lin, I agree with this organization. If you feel like > > >> somethings > > >> > > > should > > >> > > > > be transitioned from the website to the wiki, I can help with > > >> that, > > >> > but > > >> > > > for > > >> > > > > the moment I've been suggesting that new developer-focused > > >> content be > > >> > > > > placed on the wiki. > > >> > > > > > > >> > > > > On Tue, Aug 14, 2018 at 10:40 AM, Lin Yuan < > apefor...@gmail.com > > > > > >> > > wrote: > > >> > > > > > > >> > > > > > Dear MXNet community, > > >> > > > > > > > >> > > > > > As a developer, I noticed we have some developer guide > > >> scattered in > > >> > > > > > different websites (mxnet.io, cwiki): > > >> > > > > > > > >> > > > > > E.g. > > >> > > > > > > > >> > > > > > How to Create New Operators (Layers): [ > > >> > > > > > https://mxnet.incubator.apache.org/faq/new_op.html] > > >> > > > > > A Guide to Implementing Sparse Operators in MXNet Backend [ > > >> > > > > > https://cwiki.apache.org/confluence/display/MXNET/A+ > > >> > > > > > Guide+to+Implementing+Sparse+Operators+in+MXNet+Backend > > >> > > > > > ] > > >> > > > > > > > >> > > > > > When searching developer guide by keyword, only one of them > > can > > >> be > > >> > > > > returned > > >> > > > > > on either site. > > >> > > > > > > > >> > > > > > It will be more convenient for developers if all the >
Re: Mentor changes
Jason, Jim, Bob and Michael - welcome to the project! Looking forward working with you. I like Jim's suggestion to make an assessment where we are based on the maturity model and will do a first pass over the weekend. Regards, Steffen On Thu, Sep 27, 2018 at 7:37 PM Zhao, Patric wrote: > Welcome, Jason, I think MXNet will achieve the great success as same as > BigDL. > > Looking forward to working with you :) > > > > -Original Message- > > From: Hen [mailto:bay...@apache.org] > > Sent: Friday, September 28, 2018 8:23 AM > > To: dev@mxnet.incubator.apache.org > > Cc: Jim Jagielski ; Michael Wall ; > Bob > > Paulin ; Jason Dai > > Subject: Mentor changes > > > > I'd like to welcome four additional mentors (cc'd) for MXNet :) > > > > * Jason Dai; > > * Jim Jagielski; > > * Bob Paulin; and > > * Michael Wall. > > > > Suneel Marthi has also stepped back from mentoring. > > > > Thank you to each of our new mentors for joining in, and many thanks to > > Suneel for the time he's given over the last 2 years. > > > > Hen >
Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project
"Range loops aren’t always the most performant way" Do you have an example
where there's a perf difference?
"In addition, sometimes you want the index. Or maybe you want to iterate
backwards, or not start from the first, etc. Maybe you want the iterator
because you remove it from the list at the bottom of the loop Seems
like a rule for the sake of having a rule."
I should have been more clear about this point. If you're using the index
in the loop, doing reverse iteration, or not iterating from start-to-end
this inspection is smart enough to realize it and will not suggest
optimizing that type of loop. The loops that would be changes are _only_
the loops which are detected as equivalent to range-loops. Examples can be
found here:
https://clang.llvm.org/extra/clang-tidy/checks/modernize-loop-convert.html
or you can look at what's been changed in the ref PR. I've initially set
our confidence level at 'reasonable' but we could also set to 'safe' which
would further reduce the number of loops the check would apply to.
-Kellen
On Fri, Sep 28, 2018 at 3:54 PM Chris Olivier
wrote:
> -1
>
> Range loops aren’t always the most performant way. In addition, sometimes
> you want the index. Or maybe you want to iterate backwards, or not start
> from the first, etc. Maybe you want the iterator because you remove it from
> the list at the bottom of the loop Seems like a rule for the sake of
> having a rule.
>
> On Fri, Sep 28, 2018 at 2:12 AM kellen sunderland <
> kellen.sunderl...@gmail.com> wrote:
>
> > Hello MXNet devs,
> >
> > I'd like to discuss uniformly adopting C++11 range loops in the MXNet
> > project. The benefits I see are:
> >
> > * Improved C++ readability (examples below).
> > * Consistency with other languages. The range-loops are quite similar
> to
> > loops almost all other programming languages. Given we're a project that
> > supports many languages this language consistency could be positive for
> our
> > community.
> > * Consistency within the same project. Currently different authors have
> > different loops styles which hurts codebase readability.
> > * Best available performance. There are often multiple ways to write
> > loops in C++ with subtle differences in performance and memory usage
> > between loop methods. Using range-loops ensures we get the best possible
> > perf using an intuitive loop pattern.
> > * Slightly lower chance for bugs / OOB accesses when dealing with
> indexing
> > in an array for example.
> >
> > If we decide to enable this uniformly throughout the project we can
> enable
> > this policy with a simple clang-tidy configuration change. There would
> be
> > no need for reviewers to have to manually provide feedback when someone
> > uses an older C++ loops style.
> >
> > -Kellen
> >
> > Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/
> > Previous clang-tidy discussion on the list:
> >
> >
> https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E
> >
> > -
> > Examples:
> > for (auto axis_iter = param.axis.begin() ; axis_iter!= param.axis.end();
> > ++axis_iter) {
> > CHECK_LT(*axis_iter, static_cast(ishape.ndim()));
> > stride_[reverse_index] = ishape[*axis_iter];
> > ...
> > -->
> > for (int axis : param.axis) {
> > CHECK_LT(axis, static_cast(ishape.ndim()));
> > stride_[reverse_index] = ishape[axis];
> > ...
> > --
> > for (size_t i = 0; i < in_array.size(); i++) {
> > auto = in_array[i];
> > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > }
> > -->
> > for (auto & nd : in_array) {
> > pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> > }
> >
>
Re: Time out for Travis CI
Hey Zhennan, you're safe to ignore Travis failures for now. They're just informational. The reason you sometimes see quick builds and sometimes see slow builds is that we're making use of ccache in between builds. If your PR is similar to what's in master you should build very quickly, if not it's going to take a while and likely time out. If you see timeouts rebasing may speed things up. Unfortunately the timeouts are global and we're not able to increase them. I'm hoping that adding artifact caching will speed up future builds to the point that test runs and builds can be executed in under the global limit (which is ~50 minutes). -Kellen On Fri, Sep 28, 2018 at 4:05 PM Qin, Zhennan wrote: > Hi MXNet devs, > > I'm struggled with new Travis CI for a while, it always run time out for > this PR: > https://github.com/apache/incubator-mxnet/pull/12530 > > Most of the time, Jenkins CI can pass, while Travis can't be finished > within 50 minutes. For this PR, it shouldn't affect much on the build time > or unit test time. Also, I saw other PR has same problem, eg. > > > https://travis-ci.org/apache/incubator-mxnet/builds/433172088?utm_source=github_status_medium=notification > > https://travis-ci.org/apache/incubator-mxnet/builds/434404305?utm_source=github_status_medium=notification > > According to the time stamp from Travis, all passed PR are within small > code change, and can complete `make -j2` within 25s. But for timeout case, > 'make -j2' will need about 1600s. Does Travis do incremental build for each > test? Shall we increase time limit for large PR? Can we add more time stamp > for build and unites stage to help understand what's going on there? > > Thanks in advance, > Zhennan >
Re: Feedback request for new Java API
Sorry bad paste on the gist - here is the good one https://gist.github.com/gigasquid/01cd48f563db4739910592dd9ac9db20 On Fri, Sep 28, 2018 at 10:24 AM Carin Meier wrote: > +1 on option #2 > > In the case of minimizing the the overhead for code maintenance, I wanted > to suggest the option of investigating generating code from the Java > Reflection for the Java APIs. I did a quick gist from Clojure of what the > generated classes look like from the current Scala Symbol.api for > FullyConnected here > https://gist.github.com/gigasquid/01cd48f563db4739910592 > > I looks like that there is always a base Java class generated will all the > arguments. If this is the case, then there is a possibility to generate a > Java api based on this Java method automatically with just a conversion for > the Scala option and it might be reusable for all the packages. > > Not sure if it will work for this use case, but thought I would bring it > up in case it's helpful. > > - Carin > > On Fri, Sep 28, 2018 at 7:05 AM Davydenko, Denis > wrote: > >> +1 on option #2. Having clear Java interface for NDArray, from my >> perspective, would be a better experience for Java users as it won't >> require them to deal with Scala code in any capacity. Overhead of extra >> code for additional macros is justified, in my mind, as it will be >> introduced with option #1 either way, just in a different place. >> >> -- >> Thanks, >> Denis >> >> On 9/27/18, 6:14 PM, "YiZhi Liu" wrote: >> >> I vote for "2.) Leave the existing macro in place and add another >> which generates a Java friendly version" >> >> @Qing @Andrew, could you give some examples, so that people can better >> understand how it provides "best possible experience" to Java users. >> >> I have no strong preference between having JavaShape & JavaContext or >> not. >> On Thu, Sep 27, 2018 at 5:56 PM Andrew Ayres < >> andrew.f.ay...@gmail.com> wrote: >> > >> > That's not really the conversation I'm wanting to have. I want a >> discussion >> > about the macros with respect to NDArray so that we can get >> agreement on >> > our path forward with respect to implementing the NDArray wrapper. >> > >> > The design that was put forth and agreed to was for a a Java >> wrapper around >> > the Scala API. Adding a bunch of Java friendly methods inside the >> Scala >> > code would create a mess for users. Maintenance would be >> essentially the >> > same for both because either way you're going to be updating Java >> methods >> > when you make Scala changes. >> > >> > Let's please stick with the issue in the original email. >> > >> > Thanks, >> > Andrew >> > >> > On Thu, Sep 27, 2018 at 5:22 PM Qing Lan >> wrote: >> > >> > > I would like to loop this back a layer. Current, there is a >> discussion in >> > > the MXNet Scala community on the ways to implement the Java APIs. >> Currently >> > > there are two thoughts: >> > > >> > > 1. Make Scala Java Friendly (Create Java compatible methods in >> the Scala >> > > Class. such as NDArray with Java compatible constructor) >> > > 2. Make Java friendly wrappers in Scala (Andrew's explanation >> below) >> > > >> > > The first approach require minimum input from our side to >> implement >> > > however bring user a bunch of useless api they may not want to >> use. It also >> > > makes Scala package heavier. The good thing is these two packages >> require >> > > minimum maintenance cost. As a tradeoff, if any time in the >> future we want >> > > to make Java big (make Java as the primary language supported by >> MXNet), >> > > then the migration from Scala to Java will be harmful. Spark >> consider this >> > > carefully and decide not to change much on their Scala code base >> to make it >> > > more Java. >> > > >> > > The second approach will make unique NDArray, Shape, Context and >> more. The >> > > good thing about this is we can always holds a version control on >> Java. >> > > Some breaking changes on Scala may not influence much on Java. It >> did the >> > > best way to decouple the module and good for us to build unique >> pipeline >> > > for Java. The bad thing with this design is the maintenance cost >> as we need >> > > to keep two code bases, but it also make Java side easy to change >> to make >> > > it better compatible with users. >> > > >> > > Thanks, >> > > Qing >> > > >> > > On 9/27/18, 3:25 PM, "Andrew Ayres" >> wrote: >> > > >> > > Hi, >> > > >> > > Currently, we're working to implement a new Java API and >> would like >> > > some >> > > feedback from the community on an implementation detail. In >> short, the >> > > new >> > > Java API will use the existing Scala API (in a manner similar >> to how >> > > the >> > > current Clojure API works). This basically
Re: Feedback request for new Java API
+1 on option #2 In the case of minimizing the the overhead for code maintenance, I wanted to suggest the option of investigating generating code from the Java Reflection for the Java APIs. I did a quick gist from Clojure of what the generated classes look like from the current Scala Symbol.api for FullyConnected here https://gist.github.com/gigasquid/01cd48f563db4739910592 I looks like that there is always a base Java class generated will all the arguments. If this is the case, then there is a possibility to generate a Java api based on this Java method automatically with just a conversion for the Scala option and it might be reusable for all the packages. Not sure if it will work for this use case, but thought I would bring it up in case it's helpful. - Carin On Fri, Sep 28, 2018 at 7:05 AM Davydenko, Denis wrote: > +1 on option #2. Having clear Java interface for NDArray, from my > perspective, would be a better experience for Java users as it won't > require them to deal with Scala code in any capacity. Overhead of extra > code for additional macros is justified, in my mind, as it will be > introduced with option #1 either way, just in a different place. > > -- > Thanks, > Denis > > On 9/27/18, 6:14 PM, "YiZhi Liu" wrote: > > I vote for "2.) Leave the existing macro in place and add another > which generates a Java friendly version" > > @Qing @Andrew, could you give some examples, so that people can better > understand how it provides "best possible experience" to Java users. > > I have no strong preference between having JavaShape & JavaContext or > not. > On Thu, Sep 27, 2018 at 5:56 PM Andrew Ayres > wrote: > > > > That's not really the conversation I'm wanting to have. I want a > discussion > > about the macros with respect to NDArray so that we can get > agreement on > > our path forward with respect to implementing the NDArray wrapper. > > > > The design that was put forth and agreed to was for a a Java wrapper > around > > the Scala API. Adding a bunch of Java friendly methods inside the > Scala > > code would create a mess for users. Maintenance would be essentially > the > > same for both because either way you're going to be updating Java > methods > > when you make Scala changes. > > > > Let's please stick with the issue in the original email. > > > > Thanks, > > Andrew > > > > On Thu, Sep 27, 2018 at 5:22 PM Qing Lan > wrote: > > > > > I would like to loop this back a layer. Current, there is a > discussion in > > > the MXNet Scala community on the ways to implement the Java APIs. > Currently > > > there are two thoughts: > > > > > > 1. Make Scala Java Friendly (Create Java compatible methods in the > Scala > > > Class. such as NDArray with Java compatible constructor) > > > 2. Make Java friendly wrappers in Scala (Andrew's explanation > below) > > > > > > The first approach require minimum input from our side to implement > > > however bring user a bunch of useless api they may not want to > use. It also > > > makes Scala package heavier. The good thing is these two packages > require > > > minimum maintenance cost. As a tradeoff, if any time in the future > we want > > > to make Java big (make Java as the primary language supported by > MXNet), > > > then the migration from Scala to Java will be harmful. Spark > consider this > > > carefully and decide not to change much on their Scala code base > to make it > > > more Java. > > > > > > The second approach will make unique NDArray, Shape, Context and > more. The > > > good thing about this is we can always holds a version control on > Java. > > > Some breaking changes on Scala may not influence much on Java. It > did the > > > best way to decouple the module and good for us to build unique > pipeline > > > for Java. The bad thing with this design is the maintenance cost > as we need > > > to keep two code bases, but it also make Java side easy to change > to make > > > it better compatible with users. > > > > > > Thanks, > > > Qing > > > > > > On 9/27/18, 3:25 PM, "Andrew Ayres" > wrote: > > > > > > Hi, > > > > > > Currently, we're working to implement a new Java API and would > like > > > some > > > feedback from the community on an implementation detail. In > short, the > > > new > > > Java API will use the existing Scala API (in a manner similar > to how > > > the > > > current Clojure API works). This basically means that we're > making Java > > > friendly wrappers to call the existing Scala API. > > > > > > The feedback we're looking for is on the implementation of > NDArray. > > > Scala's > > > NDArray has a significant amount of code which is generated > via macros > > > and > > > we've got two viable paths
Time out for Travis CI
Hi MXNet devs, I'm struggled with new Travis CI for a while, it always run time out for this PR: https://github.com/apache/incubator-mxnet/pull/12530 Most of the time, Jenkins CI can pass, while Travis can't be finished within 50 minutes. For this PR, it shouldn't affect much on the build time or unit test time. Also, I saw other PR has same problem, eg. https://travis-ci.org/apache/incubator-mxnet/builds/433172088?utm_source=github_status_medium=notification https://travis-ci.org/apache/incubator-mxnet/builds/434404305?utm_source=github_status_medium=notification According to the time stamp from Travis, all passed PR are within small code change, and can complete `make -j2` within 25s. But for timeout case, 'make -j2' will need about 1600s. Does Travis do incremental build for each test? Shall we increase time limit for large PR? Can we add more time stamp for build and unites stage to help understand what's going on there? Thanks in advance, Zhennan
Re: [DISCUSS] Use modernized C++11 range loops uniformly throughout the project
-1
Range loops aren’t always the most performant way. In addition, sometimes
you want the index. Or maybe you want to iterate backwards, or not start
from the first, etc. Maybe you want the iterator because you remove it from
the list at the bottom of the loop Seems like a rule for the sake of
having a rule.
On Fri, Sep 28, 2018 at 2:12 AM kellen sunderland <
kellen.sunderl...@gmail.com> wrote:
> Hello MXNet devs,
>
> I'd like to discuss uniformly adopting C++11 range loops in the MXNet
> project. The benefits I see are:
>
> * Improved C++ readability (examples below).
> * Consistency with other languages. The range-loops are quite similar to
> loops almost all other programming languages. Given we're a project that
> supports many languages this language consistency could be positive for our
> community.
> * Consistency within the same project. Currently different authors have
> different loops styles which hurts codebase readability.
> * Best available performance. There are often multiple ways to write
> loops in C++ with subtle differences in performance and memory usage
> between loop methods. Using range-loops ensures we get the best possible
> perf using an intuitive loop pattern.
> * Slightly lower chance for bugs / OOB accesses when dealing with indexing
> in an array for example.
>
> If we decide to enable this uniformly throughout the project we can enable
> this policy with a simple clang-tidy configuration change. There would be
> no need for reviewers to have to manually provide feedback when someone
> uses an older C++ loops style.
>
> -Kellen
>
> Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/
> Previous clang-tidy discussion on the list:
>
> https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E
>
> -
> Examples:
> for (auto axis_iter = param.axis.begin() ; axis_iter!= param.axis.end();
> ++axis_iter) {
> CHECK_LT(*axis_iter, static_cast(ishape.ndim()));
> stride_[reverse_index] = ishape[*axis_iter];
> ...
> -->
> for (int axis : param.axis) {
> CHECK_LT(axis, static_cast(ishape.ndim()));
> stride_[reverse_index] = ishape[axis];
> ...
> --
> for (size_t i = 0; i < in_array.size(); i++) {
> auto = in_array[i];
> pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> }
> -->
> for (auto & nd : in_array) {
> pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
> }
>
Re: Feedback request for new Java API
+1 on option #2. Having clear Java interface for NDArray, from my perspective, would be a better experience for Java users as it won't require them to deal with Scala code in any capacity. Overhead of extra code for additional macros is justified, in my mind, as it will be introduced with option #1 either way, just in a different place. -- Thanks, Denis On 9/27/18, 6:14 PM, "YiZhi Liu" wrote: I vote for "2.) Leave the existing macro in place and add another which generates a Java friendly version" @Qing @Andrew, could you give some examples, so that people can better understand how it provides "best possible experience" to Java users. I have no strong preference between having JavaShape & JavaContext or not. On Thu, Sep 27, 2018 at 5:56 PM Andrew Ayres wrote: > > That's not really the conversation I'm wanting to have. I want a discussion > about the macros with respect to NDArray so that we can get agreement on > our path forward with respect to implementing the NDArray wrapper. > > The design that was put forth and agreed to was for a a Java wrapper around > the Scala API. Adding a bunch of Java friendly methods inside the Scala > code would create a mess for users. Maintenance would be essentially the > same for both because either way you're going to be updating Java methods > when you make Scala changes. > > Let's please stick with the issue in the original email. > > Thanks, > Andrew > > On Thu, Sep 27, 2018 at 5:22 PM Qing Lan wrote: > > > I would like to loop this back a layer. Current, there is a discussion in > > the MXNet Scala community on the ways to implement the Java APIs. Currently > > there are two thoughts: > > > > 1. Make Scala Java Friendly (Create Java compatible methods in the Scala > > Class. such as NDArray with Java compatible constructor) > > 2. Make Java friendly wrappers in Scala (Andrew's explanation below) > > > > The first approach require minimum input from our side to implement > > however bring user a bunch of useless api they may not want to use. It also > > makes Scala package heavier. The good thing is these two packages require > > minimum maintenance cost. As a tradeoff, if any time in the future we want > > to make Java big (make Java as the primary language supported by MXNet), > > then the migration from Scala to Java will be harmful. Spark consider this > > carefully and decide not to change much on their Scala code base to make it > > more Java. > > > > The second approach will make unique NDArray, Shape, Context and more. The > > good thing about this is we can always holds a version control on Java. > > Some breaking changes on Scala may not influence much on Java. It did the > > best way to decouple the module and good for us to build unique pipeline > > for Java. The bad thing with this design is the maintenance cost as we need > > to keep two code bases, but it also make Java side easy to change to make > > it better compatible with users. > > > > Thanks, > > Qing > > > > On 9/27/18, 3:25 PM, "Andrew Ayres" wrote: > > > > Hi, > > > > Currently, we're working to implement a new Java API and would like > > some > > feedback from the community on an implementation detail. In short, the > > new > > Java API will use the existing Scala API (in a manner similar to how > > the > > current Clojure API works). This basically means that we're making Java > > friendly wrappers to call the existing Scala API. > > > > The feedback we're looking for is on the implementation of NDArray. > > Scala's > > NDArray has a significant amount of code which is generated via macros > > and > > we've got two viable paths to move forward: > > > > 1.) Change the macro to generate Java friendly methods - To do this > > we'll > > modify the macro so that the generated methods won't have > > default/optional > > arguments. There may also have to be some changes to parameter types to > > make them Java friendly. The big advantage here is that ongoing > > maintenance > > will easier. The disadvantages are that we'll be changing the existing > > Scala NDArray Infer API (it's marked experimental) and Scala users will > > lose the ability to use the default and optional arguments. > > > > 2.) Leave the existing macro in place and add another which generates a > > Java friendly version - The biggest issue here is that we'll be > > doubling > > the number of macros that we've got to maintain. It'll become even more > > overhead once we start expanding the Java API with more classes that > > use > > generated code like
Re: Maturity Model and Graduation
On 28/09/18 11:27, kellen sunderland wrote: I'd love to see some more sustained contribution from other open source communities to help us out in this area That's not exactly the model I have seen to work. What I have seen works really well at other projects is pulling users in as committers in a scratch your own itch kind of way. For that to work you need to make it clear what contributions you need, you need to make time to coach people to become developers, you need to make your users accustomed to the way you work as early as possible. It also helps to ask users for contributions and offer mentoring help from your side along the way. I know that this is tedious work that needs a lot of motivating people, mentoring people, explaining to people, however it makes for a sustainable community of people that do the work out of self interest. http://blog.isabel-drost.de/posts/open-development-and-inner-source-for-fun-and-profit.html Isabel
Re: Maturity Model and Graduation
Hey Jim, welcome to the community. To the best of my knowledge we have not yet discussed/run a Maturity Model. My gut feel is that MXNet would come away a fairly bi-model result. My view of the project is that it's getting the Apache Way right in terms of Code, Releases, and Quality. I think the project is doing decently well with Licensing (although it's maybe a little more complex than other projects given the many required code dependencies). From my observations I would say the community often struggles with consensus building. My opinion is that the project is doing a lot right with community, especially in question answer, but is lacking in other areas such as community expansion and ownership. Independence is an area where the project is clearly behind, with almost all active committers coming from Amazon. We've had some great contributions from Intel and NVIDIA, but so far have not been able to add the members from those organizations to the IPMC for various reasons. MXNet seems to not have had much support from other open-source communities (with a notable exception of Carin who did a great job with Clojure and was made a committer/ipmc member). My impression is that the community would love to improve in this area, so the lack of progress is not due to any lack of desire on the community's part. I'd love to see some more sustained contribution from other open source communities to help us out in this area (and am hopeful we can reach out to the Julia community as an example). -Kellen On Wed, Sep 26, 2018 at 11:24 PM Jim Jagielski wrote: > As a newly "minted" mentor, I'm getting my feet wet on determining where > the project is and where it needs to go in order to be ready for > graduation... > > Has the project run the Maturity Model against itself? How do we stack up? > What areas of improvement could we benefit from (this might be independent > of what the MatModel sez, btw. If you have ideas on where we could be > working and collaborating better, please bring them up!)? > > Cheers!
[DISCUSS] Use modernized C++11 range loops uniformly throughout the project
Hello MXNet devs,
I'd like to discuss uniformly adopting C++11 range loops in the MXNet
project. The benefits I see are:
* Improved C++ readability (examples below).
* Consistency with other languages. The range-loops are quite similar to
loops almost all other programming languages. Given we're a project that
supports many languages this language consistency could be positive for our
community.
* Consistency within the same project. Currently different authors have
different loops styles which hurts codebase readability.
* Best available performance. There are often multiple ways to write
loops in C++ with subtle differences in performance and memory usage
between loop methods. Using range-loops ensures we get the best possible
perf using an intuitive loop pattern.
* Slightly lower chance for bugs / OOB accesses when dealing with indexing
in an array for example.
If we decide to enable this uniformly throughout the project we can enable
this policy with a simple clang-tidy configuration change. There would be
no need for reviewers to have to manually provide feedback when someone
uses an older C++ loops style.
-Kellen
Reference PR: https://github.com/apache/incubator-mxnet/pull/12356/
Previous clang-tidy discussion on the list:
https://lists.apache.org/thread.html/b0ae5a9df5dfe0d9074cb2ebe432264db4fa2175b89fa43a5f6e36be@%3Cdev.mxnet.apache.org%3E
-
Examples:
for (auto axis_iter = param.axis.begin() ; axis_iter!= param.axis.end();
++axis_iter) {
CHECK_LT(*axis_iter, static_cast(ishape.ndim()));
stride_[reverse_index] = ishape[*axis_iter];
...
-->
for (int axis : param.axis) {
CHECK_LT(axis, static_cast(ishape.ndim()));
stride_[reverse_index] = ishape[axis];
...
--
for (size_t i = 0; i < in_array.size(); i++) {
auto = in_array[i];
pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
}
-->
for (auto & nd : in_array) {
pre_temp_buf_.emplace_back(nd.shape(), nd.ctx(), true, nd.dtype());
}
