KEMBAR78
fix normalize mean error bug by nicklhy · Pull Request #15539 · apache/mxnet · GitHub
Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Conversation

@nicklhy
Copy link
Contributor

@nicklhy nicklhy commented Jul 15, 2019

The normalize operator may produce wrong result when setting the mean arg with a single float val.

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 for the fix : )

Copy link
Contributor

@ChaiBapchya ChaiBapchya left a comment

Choose a reason for hiding this comment

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

Wow! How did we miss this?
Good find! Any test you wrote to verify this?

@karan6181
Copy link
Contributor

@mxnet-label-bot add [Operator, pr-awaiting-response]

@marcoabreu marcoabreu added Operator pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Jul 15, 2019
@nicklhy
Copy link
Contributor Author

nicklhy commented Jul 16, 2019

Wow! How did we miss this?
Good find! Any test you wrote to verify this?

@ChaiBapchya We are upgrading MXNet from 1.3 to 1.5 in our deployment env these days and notice some accuracy drop of our models. This bug seems to exist for at least a few months. Maybe we should add more tests to avoid similar bugs in the future.

@wkcn
Copy link
Member

wkcn commented Jul 18, 2019

Hi @nicklhy , could you please add an unittest in test_image_normalize ?

Thank you!

@nicklhy
Copy link
Contributor Author

nicklhy commented Jul 19, 2019

@wkcn Just added some tests for scalar mean/std support in b952e91.

@wkcn
Copy link
Member

wkcn commented Jul 19, 2019

@nicklhy Great work! Thank you so much!

@wkcn wkcn added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Jul 20, 2019
@wkcn wkcn merged commit 8f5930b into apache:master Jul 20, 2019
@nicklhy nicklhy deleted the fix_normalize_bug branch July 22, 2019 01:21
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
* fix normalize mean error bug

* add scalar mean/std tests for image_normalize
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Operator pr-awaiting-merge Review and CI is complete. Ready to Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants