mxnet-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] leezu opened a new pull request #11764: Fix MXNET_FORCE_ADDTAKEGRAD
Date Sun, 15 Jul 2018 17:57:14 GMT
leezu opened a new pull request #11764: Fix MXNET_FORCE_ADDTAKEGRAD
URL: https://github.com/apache/incubator-mxnet/pull/11764
 
 
   ## Description ##
   A logic error was introduced by the second commit in (#11316)  _Read MXNET_FORCE_ADDTAKEGRAD
to a static variable_ https://github.com/apache/incubator-mxnet/pull/11316/commits/a90048569abfe3341cebe25417ec9de0446b94cc
and `MXNET_FORCE_ADDTAKEGRAD=0` forced the use of AddTakeGrad kernel instead of `MXNET_FORCE_ADDTAKEGRAD=1`.
   
   This means forcing the AddTakeGrad kernel was the default behavior. While the kernel may
be slightly slower it works correctly. It may be better to err on the side of speed instead
of correctness and keep `MXNET_FORCE_ADDTAKEGRAD=1` the default (which it was for the last
weeks due to above bug)?
   
   I'm using a separate fork of mxnet to which I didn't add the second commit so that the
bug went unnoticed.
   
   ## 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)
   - [X] 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)
   - [X] 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 http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
   - [X] To the my best knowledge, examples are either not affected by this change, or have
been fixed to be compatible with this change
   
   ### Changes ###
   - [X] Fix MXNET_FORCE_ADDTAKEGRAD

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Mime
View raw message