You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/07/01 05:07:01 UTC
[GitHub] [incubator-mxnet] sxjscience opened a new pull request #18648: [v1.7.x] [Backport]add zero grad for npi_unique (#18080)
sxjscience opened a new pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648
## Description ##
(Brief description on what this PR is about)
## Checklist ##
### Essentials ###
Please feel free to remove inapplicable items for your PR.
- [ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) created (except PRs with tiny changes)
- [ ] Changes are complete (i.e. I finished coding on this PR)
- [ ] All changes have test coverage:
- Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
- Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
- Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
- [ ] Code is well-documented:
- For user-facing API changes, API doc string has been updated.
- For new C++ functions in header files, their functionalities and arguments are documented.
- For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
- Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
- [ ] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
### Changes ###
- [ ] Feature1, tests, (and when applicable, API doc)
- [ ] Feature2, tests, (and when applicable, API doc)
## Comments ##
- If this change is a backward incompatible change, why must this change be made.
- Interesting edge cases to note here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652510821
@sandeep-krishnamurthy @ciyongch I created this PR to track how many numpy commits would potentially go into v1.7.0 (not all of them should go into 1.7.0):
We may
- Ignore all the FFI commits
- Ignore commits that add new operators
However, we should add all the bug fixes
Apart from the following two commits
- #18523
- #18080
We will also need
- https://github.com/apache/incubator-mxnet/pull/18250
- https://github.com/apache/incubator-mxnet/pull/17788
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652736178
Thanks a lot @sxjscience for the great help to filter the necessary PRs targeting at 1.7 release and @sandeep-krishnamurthy for the valuable comments :)
When you're deciding to backport these PRs, are there any additional cases to make sure they're working expected at your side?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sandeep-krishnamurthy commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sandeep-krishnamurthy commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652532059
> @sandeep-krishnamurthy @ciyongch I created this PR to track how many numpy commits would potentially go into v1.7.0 (not all of them should go into 1.7.0):
>
> We may
>
> * Ignore all the FFI commits
> * Ignore commits that add new operators
>
> However, we should add all the bug fixes
>
> Apart from the following two commits
>
> * #18523
> * #18080
>
> We will also need
>
> * #18250
> * #17788
Thank you @sxjscience - Can you please help with back port of these 4 bug fix PRs?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652792263
Sure, that one should be fine, how about the rest of two PRs? May I know how much effort and time you'll need to backport them? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #18648: [v1.7.x] [Backport]add zero grad for npi_unique (#18080)
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652193277
Hey @sxjscience , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [windows-gpu, centos-cpu, edge, windows-cpu, unix-gpu, miscellaneous, unix-cpu, website, clang, sanity, centos-gpu]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652267739
@ciyongch I find that there are lots of numpy stuffs here. This is the initial attempt for backporting some commits. One issue is #17645, in which I'm not sure whether to port or not.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652355926
As numpy operator feature is in a quite active development in current master branch and mainly targeting on 2.0 release. It'll be fine if the backport patch is small and controllable at this time frame, but given the current one is introducing substantial modifications (+16,079 −1,160 code changes in 192 files), which is really a big concern whether this patch can help to make it solid enough while not introducing any other numpy issue. I agree to include existing fixes as much as possible, but this one is kind of complicated and need further confirmation. What do you think @szha @sandeep-krishnamurthy ?
[Quote](https://github.com/apache/incubator-mxnet/issues/18641#issuecomment-651498541) from @sandeep-krishnamurthy
>+1 to mark this as experimental as it was not stable in 1.6 or advertised as will be stable in v1.7. We are mainly looking at numpy ops in 2.0.
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] [Backport]add zero grad for npi_unique (#18080)
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652235614
@ciyongch I traced the commits that are before 743bbcb and these are the initial list of commits.
There are two remaining commits like:
- https://github.com/apache/incubator-mxnet/pull/18250
- https://github.com/apache/incubator-mxnet/pull/18251
Also, there is one commit that I'm not sure whether it should be backported or not:
- https://github.com/apache/incubator-mxnet/pull/17645
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sandeep-krishnamurthy commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sandeep-krishnamurthy commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652767947
@sxjscience when will it be possible to have a Backport PRs? @ciyongch what is the timeline you had planned for rc0?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652272413
Thanks @sxjscience for the effort.
It seems that this backport introduce **huge code changes** which will be a big concern to the current stable code base especially after code freeze, then I'm more preferred NOT to include this at 1.7 release, what do you think? Copy @szha as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652784828
@ciyongch I'm going to test v1.7 after this backporting PR (https://github.com/apache/incubator-mxnet/pull/18653) is merged. WOuld you merge that one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sandeep-krishnamurthy commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sandeep-krishnamurthy commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652480776
First of all thank you very much @sxjscience for this tremendous effort in backporting PRs.
I am really concerned with addition of new experimental features in the last minute. 1.7 was stable with many required functionality for users and no known breaking new changes compared to v1.6.
I still believe all numpy related changes in these commits should go in a separate release and not block v1.7 as I believe this is not a stable feature set expected by users or advertised 2 months ago before code freeze as v1.7 feature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652796279
@TaoLv helped to merge that PR, please help to check if the current code base works expected or not :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience closed pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience closed pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience edited a comment on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience edited a comment on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652235614
@ciyongch I traced the commits that are before 743bbcb and these are the initial list of commits.
There are two remaining commits like:
- ~~https://github.com/apache/incubator-mxnet/pull/18250~~ (backported)
- https://github.com/apache/incubator-mxnet/pull/18251
Also, there is one commit that I'm not sure whether it should be backported or not:
- https://github.com/apache/incubator-mxnet/pull/17645
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652784885
Close this PR now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sxjscience commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652295990
In addition, we do have the ability to test whether the numpy APIs are stable or not by checking the tests in GluonNLP: https://github.com/dmlc/gluon-nlp/tree/numpy/tests
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Xingjian SHI <xs...@connect.ust.hk>
Sent: Wednesday, July 1, 2020 2:04:42 AM
To: apache/incubator-mxnet <re...@reply.github.com>; apache/incubator-mxnet <in...@noreply.github.com>
Cc: Mention <me...@noreply.github.com>
Subject: Re: [apache/incubator-mxnet] [v1.7.x] Backport some numpy features + fixes (#18648)
I think we should try to include as many numpy fixes as possible. The numpy feature was introduced in 1.6.0 and we should have a rather stable version in 1.7.0.
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: ciyong <no...@github.com>
Sent: Wednesday, July 1, 2020 1:25:12 AM
To: apache/incubator-mxnet <in...@noreply.github.com>
Cc: Xingjian SHI <xs...@connect.ust.hk>; Mention <me...@noreply.github.com>
Subject: Re: [apache/incubator-mxnet] [v1.7.x] Backport some numpy features + fixes (#18648)
Thanks @sxjscience<https://github.com/sxjscience> for the effort.
It seems that this backport introduce huge code changes which will be a big concern to the current stable code base especially after code freeze, then I'm more preferred NOT to include this at 1.7 release, what do you think? Copy @szha<https://github.com/szha> as well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652272413>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHQH3SZPF2V7KKMFANCJE3RZLXGRANCNFSM4ONBA5YQ>.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] ciyongch commented on pull request #18648: [v1.7.x] Backport some numpy features + fixes
Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #18648:
URL: https://github.com/apache/incubator-mxnet/pull/18648#issuecomment-652773680
Hi @sandeep-krishnamurthy , actually I was planning to drop rc0 before numpy related issues were raised.
@sxjscience has already submitted partial PRs listed as above in https://github.com/apache/incubator-mxnet/pull/18653. The remaining PRs are (#18250 and #18523), it would be great if the rest of PRs can be backported within 48h. @sxjscience can you please help to give an estimate for the remaining effort, or we might need to cut off some fixes/PRs if it's uncertain or taking too much time. What do you think? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org