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

Get rid of monkey patching in LossScaler overflow handling - #18959

Merged
ptrendx merged 1 commit into
apache:masterfrom
mk-61:pr-no-monkeys-loss-scaler
Aug 19, 2020
Merged

Get rid of monkey patching in LossScaler overflow handling#18959
ptrendx merged 1 commit into
apache:masterfrom
mk-61:pr-no-monkeys-loss-scaler

Conversation

@mk-61

@mk-61 mk-61 commented Aug 18, 2020

Copy link
Copy Markdown
Contributor

Description

Current overflow checking in AMP LossScaler uses monkey patching and closures, which doesn't mix well with pickling, causing #16858. This change implements similar logic without monkey patching.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@ptrendx
@mxnet-bot

mxnet-bot commented Aug 18, 2020

Copy link
Copy Markdown

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: [centos-cpu, clang, centos-gpu, unix-gpu, edge, miscellaneous, sanity, unix-cpu, windows-gpu, website, windows-cpu]

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 commented Aug 19, 2020

Copy link
Copy Markdown
Contributor Author

@mxnet-bot run ci [windows-gpu]
@mxnet-bot

mxnet-bot commented Aug 19, 2020

Copy link
Copy Markdown

Jenkins CI successfully triggered : [windows-gpu]
@mk-61

mk-61 commented Aug 19, 2020

Copy link
Copy Markdown
Contributor Author

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

mxnet-bot commented Aug 19, 2020

Copy link
Copy Markdown

Jenkins CI successfully triggered : [unix-cpu]
ptrendx
ptrendx approved these changes Aug 19, 2020
View reviewed changes

@ptrendx ptrendx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM
@ptrendx
ptrendx merged commit 22d69ef into apache:master Aug 19, 2020
@ptrendx ptrendx mentioned this pull request Aug 20, 2020
Closed
mk-61 added a commit to mk-61/incubator-mxnet that referenced this pull request Aug 20, 2020
@mk-61 @vcherepanov-nv
Get rid of monkey patching in LossScaler overflow handling (apache#18959
4d3e180 
)

Co-authored-by: Vladimir Cherepanov <vcherepanov@nvidia.com>
ptrendx pushed a commit that referenced this pull request Aug 24, 2020
@mk-61 @vcherepanov-nv
Get rid of monkey patching in LossScaler overflow handling (#18959) (#…
dfefe87 
…18973)

Co-authored-by: Vladimir Cherepanov <vcherepanov@nvidia.com>

Co-authored-by: Vladimir Cherepanov <vcherepanov@nvidia.com>
@mk-61 mk-61 mentioned this pull request Aug 26, 2020
Closed
@mk-61 mk-61 mentioned this pull request Oct 5, 2020
Open
chinakook pushed a commit to chinakook/mxnet that referenced this pull request Aug 11, 2021
@mk-61 @vcherepanov-nv @chinakook
Get rid of monkey patching in LossScaler overflow handling (apache#18959
afe1023 
)

Co-authored-by: Vladimir Cherepanov <vcherepanov@nvidia.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants

@mk-61 @mxnet-bot @ptrendx @vcherepanov-nv