-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Backporting recent mx.np changes to 1.7 branch #18641
Comments
Hi @leezu, given the fact that the current numpy operator is still in active development, there could be more defects/bugs as including more new functionalities/features in v1.7. Thus it's uncertain about how longer it will take to backport these numpy bug fixes/features from master to v1.7, I suggest to mark numpy operator as experimental feature in v1.7 release, and decide a cut off day (for example 24h or 48h) to include the fixes that are available, and moving the 1.7 release process forward, what do you think? |
@sandeep-krishnamurthy @ciyongch How about to port these two bug-fixes to 1.7? |
@sandeep-krishnamurthy @ciyongch I agree on marking numpy as experimental. Still, we'd really like the bug fixes that @sxjscience requests backported, so that we could use the related functionality and present them at the upcoming KDD in the summer. |
Thanks @sandeep-krishnamurthy @szha , then let's take it as the experimental feature in v1.7 release. @sxjscience could you please help to backport these two PRs and tag me on the new PR? Then we'll move forward with rc0 tag and the rest of release process when they're get merged. |
Hi @sxjscience may I know if you're going to backport the above two PR into 1.7 as @szha suggested? We're waiting for them to be merged and tag rc0 now, thanks! |
Thanks @sxjscience for your prompt help on this. I've already ping the author for #18523, do you need any help for this PR? |
@ciyongch The problem is that I think there are more numpy PRs that should may need to be backported... |
Wow... can you help to evaluate how many PRs are related to enable this feature? Then we can decide the time needed and whether to include them or not? Thanks! |
Attempted to backport some changes in #18648. However, there are lots of changes in numpy.
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: ciyong <notifications@github.com>
Sent: Tuesday, June 30, 2020 10:48:01 PM
To: apache/incubator-mxnet <incubator-mxnet@noreply.github.com>
Cc: Xingjian SHI <xshiab@connect.ust.hk>; Mention <mention@noreply.github.com>
Subject: Re: [apache/incubator-mxnet] Backporting recent mx.np changes to 1.7 branch (#18641)
Wow... can you help to evaluate how many PRs are related to enable this feature? Then we can decide the time needed and whether to include them or not? Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18641 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHQH3ST6THQMB37W5WKVNTRZLEZDANCNFSM4OLXW63Q>.
|
Thanks @sxjscience and @BenjaminCHEN2016 for the great effort to backport the requested fixes to v1.7, now all of them were merged into v1.7.x via the PR #18653 and #18649. I will proceed with rc0 when the night build test passed. |
Should we close this issue as the minimum fixes were backport to v1.7 release. Or we can make a complete fix in the upcoming 1.8 release? @leezu @samskalicky |
Yes, if all the fixes are in v1.x we can close this issue. Given where we are in the release process for v1.8 it would be preferred to have these fixes in the v1.8 so we can have a complete fix in the v1.8 release. But if not, we can still close the issue as long as its fixed in v1.x and will make it into the next release (ie. v1.9) |
Description
master branch contains many fixes for various numpy operators included in the 1.7 release. While they are experimental in 1.7, it would be helpful to include the fixes. For example, I tested running GluonNLP NumPy version with the 1.7 branch and some unittests are failing.
For example,
MXNetError: Operator _npi_unique is non-differentiable because it didn't register FGradient attribute
which was fixed in #18074 is not part of 1.7To fix this issue, one could copy over all numpy operator related C++ and Python files from the master branch to the 1.x and 1.7 branches.
https://github.com/leezu/gluon-nlp/runs/820282077?check_suite_focus=true
cc: @ciyongch @sxjscience @yzhliu
The text was updated successfully, but these errors were encountered: