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

[v1.8.x][BUGFIX] Fix MKLDNN BatchNorm with even number of channels (#19150) #19299 #19425 #19428

Merged
merged 3 commits into from
Oct 29, 2020

Conversation

akarbown
Copy link
Contributor

Description

It's the change (#19299) ported form mxnet 1.7.

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

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

Even number of channels results in data reordering before batch
norm operation. Therefore, if BatchNorm data array is view of
another array and the data is stored in MKLDNN format, the data
needs to be converted to the default format.
@mxnet-bot
Copy link

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

@akarbown akarbown changed the base branch from master to v1.8.x October 26, 2020 17:02
@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 26, 2020
@akarbown
Copy link
Contributor Author

@mxnet-bot run ci [unix-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu]

@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 27, 2020
Copy link
Member

@wkcn wkcn left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@TaoLv TaoLv changed the title [BUGFIX] Fix MKLDNN BatchNorm with even number of channels (#19150) #19299 #19425 [v1.8.x][BUGFIX] Fix MKLDNN BatchNorm with even number of channels (#19150) #19299 #19425 Oct 29, 2020
@TaoLv
Copy link
Member

TaoLv commented Oct 29, 2020

@samskalicky Do you want to confirm this is needed for the release?

@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-awaiting-review PR is waiting for code review pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 29, 2020
@lanking520 lanking520 added pr-awaiting-merge Review and CI is complete. Ready to Merge pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-awaiting-merge Review and CI is complete. Ready to Merge labels Oct 29, 2020
@samskalicky
Copy link
Contributor

Bug fixes are still ok until we finish the license issues.

Can someone confirm this fix has already been ported to the 1.x branch already before we merge this?

@lanking520 lanking520 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 29, 2020
@akarbown
Copy link
Contributor Author

Bug fixes are still ok until we finish the license issues.

Can someone confirm this fix has already been ported to the 1.x branch already before we merge this?

I'll create PR with that fix for 1.x today.

@akarbown
Copy link
Contributor Author

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

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, windows-gpu]

@samskalicky
Copy link
Contributor

Thanks @akarbown for creating #19445 on v1.x too!

@samskalicky samskalicky merged commit 3e02554 into apache:v1.8.x Oct 29, 2020
@akarbown akarbown deleted the v1.8_batchnorm_is_view branch November 16, 2020 10:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-work-in-progress PR is still work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants