Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport] Fix decorrelated batch normalization when groups ≠ 1 #7825

Merged
merged 2 commits into from
Sep 3, 2019

Conversation

hvy
Copy link
Member

@hvy hvy commented Jul 29, 2019

Backport of #7707

Fix decorrelated batch normalization when groups ≠ 1
@hvy hvy added backport Pull request that is backported from a more recent version. cat:bug Bug report or fix. labels Jul 29, 2019
@hvy
Copy link
Member Author

hvy commented Jul 29, 2019

[automatic post] Jenkins, test this please.

@hvy
Copy link
Member Author

hvy commented Jul 29, 2019

We need to backport this PR first #7226 ?

@toslunar
Copy link
Member

#7226

chainer.backend.copyto is relevant but Variable.copydata is not.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit e532541, target branch v6) failed with status FAILURE.

@toslunar
Copy link
Member

I've not understood the difference between

if isinstance(value, chainerx.ndarray):
value_view = chainerx.to_numpy(value, copy=False)
numpy.copyto(value_view, dataset)

and
if isinstance(dst, chainerx.ndarray):
dst[...] = _chainerx._array_to_chainerx(src, dst.device)

@hvy
Copy link
Member Author

hvy commented Jul 29, 2019

The former indeed seems broken #7829

@niboshi
Copy link
Member

niboshi commented Jul 29, 2019

TODO: Backport #7828 after this.

@hvy
Copy link
Member Author

hvy commented Jul 30, 2019

chainer.backend.copyto is relevant but Variable.copydata is not.

@toslunar Are there concerns with backporting #7226 ? I'd like to do so otherwise.

@niboshi
Copy link
Member

niboshi commented Aug 13, 2019

ping

@toslunar
Copy link
Member

I made #7934. Whether to backport the other part of #7226 might be discussed later.

@niboshi
Copy link
Member

niboshi commented Aug 15, 2019

#7934 merged.

@hvy
Copy link
Member Author

hvy commented Aug 19, 2019

@toslunar could you take another look at this backport now that your PR has been merged?

@toslunar
Copy link
Member

Jenkins & flexCI, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 7d8cf58, target branch v6) succeeded!

@toslunar toslunar added this to the v6.4.0 milestone Sep 3, 2019
Copy link
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

LGTM

@toslunar toslunar added the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Sep 3, 2019
@mergify mergify bot merged commit 56a11c4 into chainer:v6 Sep 3, 2019
@hvy hvy deleted the bp-7707-fix-dbn-groups branch September 3, 2019 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Pull request that is backported from a more recent version. cat:bug Bug report or fix. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants