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

Refactor test_batch_normalization #4224

Merged
merged 3 commits into from Jan 25, 2018

Conversation

niboshi
Copy link
Member

@niboshi niboshi commented Jan 24, 2018

Refactored test_batch_normalization to use testing.backend.inject_backend_tests.

@niboshi niboshi added the cat:test Test or CI related. label Jan 24, 2018
@niboshi niboshi force-pushed the refactor-test-batch-normalization branch from 3391af7 to 2155c0a Compare January 24, 2018 08:22
@niboshi niboshi force-pushed the refactor-test-batch-normalization branch from 51fa215 to 1899ff3 Compare January 24, 2018 17:06
Copy link
Member

@kmaehashi kmaehashi left a comment

Choose a reason for hiding this comment

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

LGTM except for one minor comment!



def _batch_normalization(expander, gamma, beta, x, mean, var):
def _to_noncontiguous(arrays):
Copy link
Member

Choose a reason for hiding this comment

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

I feel the name _to_noncontiguous is a bit misleading (as the resulting array is F-contiguous)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. _to_fcontiguous?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@kmaehashi kmaehashi added the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Jan 25, 2018
@kmaehashi kmaehashi merged commit 6dd7e51 into chainer:master Jan 25, 2018
@kmaehashi
Copy link
Member

LGTM!

@kmaehashi kmaehashi added this to the v4.0.0b4 milestone Jan 25, 2018
@niboshi niboshi deleted the refactor-test-batch-normalization branch January 25, 2018 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:test Test or CI related. 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.

None yet

2 participants