Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

AMP support for Numpy ops #19036

Merged
merged 8 commits into from
Oct 2, 2020
Merged

AMP support for Numpy ops #19036

merged 8 commits into from
Oct 2, 2020

Conversation

mk-61
Copy link
Contributor

@mk-61 mk-61 commented Aug 29, 2020 •
edited
Loading

Description

  1. Add all registered ops (mostly Numpy) to AMP lists
  2. Removed "skipped" attribute from AMP tests
  3. Some change in AMP monkey patching, since NumPy ops require a bit different handling.

@mk-61 mk-61 requested a review from szha as a code owner August 29, 2020 00:23
@mxnet-bot
Copy link

Hey @mk-61 , 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: [sanity, centos-gpu, miscellaneous, windows-cpu, windows-gpu, website, unix-cpu, centos-cpu, edge, clang, unix-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.

@mk-61 mk-61 force-pushed the pr-amp-numpy-ops branch 2 times, most recently from 42f1ae0 to fa05888 Compare August 30, 2020 20:29
@mk-61 mk-61 changed the title [WIP] AMP lists for numpy ops [WIP] AMP support for Numpy ops Aug 30, 2020
@mk-61 mk-61 force-pushed the pr-amp-numpy-ops branch 2 times, most recently from 35efcec to 6a991a2 Compare August 30, 2020 22:27
@mk-61 mk-61 force-pushed the pr-amp-numpy-ops branch 2 times, most recently from 80a3102 to 48e7bfe Compare September 14, 2020 19:38
@lanking520 lanking520 added the pr-work-in-progress PR is still work in progress label Sep 14, 2020
@mk-61 mk-61 force-pushed the pr-amp-numpy-ops branch 3 times, most recently from 3ca306f to 129328c Compare September 17, 2020 20:02
@mk-61
Copy link
Contributor Author

mk-61 commented Sep 18, 2020

@mxnet-bot run ci [centos-cpu, unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu, centos-cpu]

@szha szha requested a review from ptrendx September 22, 2020 04:58
@mk-61 mk-61 force-pushed the pr-amp-numpy-ops branch 2 times, most recently from 4abdc6e to 1a31939 Compare September 23, 2020 02:49
@mk-61
Copy link
Contributor Author

mk-61 commented Sep 23, 2020

@mxnet-bot run ci [all]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-cpu, unix-gpu, windows-gpu, sanity, clang, centos-gpu, website, centos-cpu, edge, unix-cpu, miscellaneous]

@mk-61 mk-61 changed the title [WIP] AMP support for Numpy ops AMP support for Numpy ops Sep 24, 2020
@szha
Copy link
Member

szha commented Sep 24, 2020 

[2020-09-24T01:42:24.553Z] ************* Module mxnet.contrib.amp.loss_scaler

[2020-09-24T01:42:24.553Z] python/mxnet/contrib/amp/loss_scaler.py:45:4: C0111: Missing method docstring (missing-docstring)

[2020-09-24T01:42:24.553Z] python/mxnet/contrib/amp/loss_scaler.py:52:8: W0105: String statement has no effect (pointless-string-statement)

@mk-61
Copy link
Contributor Author

mk-61 commented Sep 24, 2020

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@mk-61
Copy link
Contributor Author

mk-61 commented Sep 25, 2020

@mxnet-bot run ci [centos-cpu, unix-cpu, unix-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, unix-cpu, centos-cpu]

@mk-61
Copy link
Contributor Author

mk-61 commented Sep 25, 2020

@mxnet-bot run ci [centos-cpu, unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu, centos-cpu]

@mk-61
Copy link
Contributor Author

mk-61 commented Sep 28, 2020

@mxnet-bot run ci [windows-gpu, macosx-x86_64]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-gpu]

sxjscience
sxjscience approved these changes Sep 29, 2020
View reviewed changes
Copy link
Member

@sxjscience sxjscience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the PR and it looks good.

@sxjscience sxjscience added AMP v2.0 and removed pr-work-in-progress PR is still work in progress labels Sep 29, 2020
ptrendx
ptrendx approved these changes Oct 1, 2020
View reviewed changes
Copy link
Member

@ptrendx ptrendx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo this change in the reason for the skipped test - could you clarify that?

@szha szha merged commit 39ea683 into apache:master Oct 2, 2020
@mk-61 mk-61 mentioned this pull request Oct 5, 2020
Open
@ptrendx ptrendx mentioned this pull request Oct 13, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sxjscience sxjscience sxjscience approved these changes

@ptrendx ptrendx ptrendx approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
AMP v2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mk-61 @mxnet-bot @szha @sxjscience @ptrendx @lanking520