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
Merged

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

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

@akarbown
Fix MKLDNN BatchNorm with even number of channels (apache#19150)
77ccf78 
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.
@akarbown
Add or updated test to verify Batchnorm odd & even number of channels
db8ea9c
@akarbown
Fix for Batchnorm odd & even chnls number context
4ad2c9b
@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
wkcn
wkcn approved these changes Oct 29, 2020
View reviewed changes
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.
Reviewers

@wkcn wkcn wkcn approved these changes

@samskalicky samskalicky samskalicky approved these changes

@aaronmarkham aaronmarkham Awaiting requested review from aaronmarkham

@eric-haibin-lin eric-haibin-lin Awaiting requested review from eric-haibin-lin

@gigasquid gigasquid Awaiting requested review from gigasquid

@iblislin iblislin Awaiting requested review from iblislin

@leezu leezu Awaiting requested review from leezu

@marcoabreu marcoabreu Awaiting requested review from marcoabreu

@nswamy nswamy Awaiting requested review from nswamy

@sergeykolychev sergeykolychev Awaiting requested review from sergeykolychev

@szha szha Awaiting requested review from szha

@yzhliu yzhliu Awaiting requested review from yzhliu

Assignees
No one assigned
Labels
pr-work-in-progress PR is still work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@akarbown @mxnet-bot @TaoLv @samskalicky @wkcn @lanking520