[MXNET-1258]fix unittest for ROIAlign Operator #13609

wkcn · 2018-12-11T03:22:31Z

Description

Fixes #11064
It passes the test whose MXNET_TEST_SEED=35650200.

The old unittest for ROIAlign computes in float64, so there is some float precision problem.
Fixes #11064

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http:https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

use float32 in the unittest for ROIAlign OP
set atol to 1e-5 in the unittest for ROIAlign OP

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

wkcn · 2018-12-11T05:03:05Z

It seems there is a problem in Windows-CPU CI.

WindowsError: [Error 5] Access is denied: 'c:\\Anaconda3\\envs\\py2\\Lib\\site-packages\\six-1.12.0.dist-info\\RECORD.pip'
You are using pip version 9.0.1, however version 18.1 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.
Usage: python.exe -m nose [options]
python.exe -m nose: error: no such option: --with-timer
Error running unittest

roywei · 2018-12-12T00:17:25Z

@mxnet-label-bot add[CI, Flaky, pr-awaiting-review]

…to fix_roi_align_test

anirudhacharya · 2018-12-17T20:45:18Z

can you run the test >500 times and paste the results here to ensure that the test is no longer flaky.

wkcn · 2018-12-17T23:20:40Z

@anirudhacharya Thank you.
I wrote the test code and tested it on CPU and GPU.
CPU: 1000+ times
GPU: 1000+ times

It works fine.
I hope it will be no longer flaky :)

anirudhacharya

there are a few unnecessary newlines.

tests/python/unittest/test_operator.py

anirudhacharya · 2018-12-19T02:21:29Z

tests/python/unittest/test_operator.py

- y = max(0.0, y)
+ return T(0.0), []
+ x = T(max(0.0, x))
+ y = T(max(0.0, y))
 x_low = int(x)


why not typecast x_low to np.float32 here itself and remove the many typecasting statements below?

The type of x_low is int in the C implementation.

In Python, the default type of float is float64, however the float type in the C implementation is float32.
If the types are not consistent, there will be calculation error.

So I use T to typecast some variables to np.float32 to keep the type consistency between C and Python unittest.

anirudhacharya · 2018-12-19T02:25:52Z

tests/python/unittest/test_operator.py


- out[r, c, ph, pw] = val * 1.0 / count
+ out[r, c, ph, pw] = val / count
+ assert out.dtype == T, out.dtype


can you add a more descriptive error message

Thanks! I will add more detail.

Roshrini · 2019-01-02T19:04:06Z

@anirudhacharya Can you take a look again?

anirudhacharya · 2019-01-02T21:20:59Z

LGTM

anirudhacharya · 2019-01-14T22:38:33Z

@wkcn please rebase and resolve conflicts

@mxnet-label-bot update [pr-awaiting-merge]

wkcn · 2019-01-15T00:43:53Z

@anirudhacharya In my own project, there is also a ROIAlignOP unittest, and I found that nvcc will change the order of operators because of compiler optimization, causing the problem of float precision. So please not merge the PR temporarily, and I will check it. [link]

stu1130 · 2019-01-15T23:07:41Z

@mxnet-label-bot update [pr-work-in-progress]
let us know once you've done the check

wkcn · 2019-01-16T00:17:55Z

@stu1130 Thanks!

wkcn · 2019-01-17T04:12:26Z

test fail in test activation
#13915

…into fix_roi_align_test

wkcn · 2019-01-17T15:54:21Z

@anirudhacharya @stu1130
Hi! I think the PR is finished.
Since nvcc will change the order of operator, I increase the tolerance to 1e-3.

sandeep-krishnamurthy · 2019-01-28T23:23:12Z

@anirudhacharya - Can you please a look at this PR?

anirudhacharya · 2019-01-29T08:44:10Z

@sandeep-krishnamurthy have already approved this PR, above.

vandanavk

LGTM.

Could you add "Fixes #11064" in the PR description? This will automatically close the issue when the PR is merged

vandanavk · 2019-02-05T19:27:44Z

@mxnet-label-bot update [Test, pr-awaiting-merge]

* fix roi align test * retrigger unittest * add more test detail for ROIAlign test * remove url in test_op_roi_align * remove blank line in test_op_roi_align in test_operator * merge master * Update test_operator.py * retrigger CI

fix roi align test

1be6c50

wkcn changed the title ~~fix unittest for ROIAlign Operator~~ [MXNET-1258]fix unittest for ROIAlign Operator Dec 11, 2018

retrigger unittest

aaa9d39

marcoabreu added CI Flaky pr-awaiting-review PR is waiting for code review labels Dec 12, 2018

jlcontreras mentioned this pull request Dec 13, 2018

Disable flaky test: test_op_roi_align #13546

Closed

Merge branch 'master' of https://github.com/apache/incubator-mxnet in…

6f3b55d

…to fix_roi_align_test

anirudhacharya reviewed Dec 19, 2018

View reviewed changes

wkcn added 3 commits December 19, 2018 12:25

add more test detail for ROIAlign test

d0ada04

remove url in test_op_roi_align

afcfe76

remove blank line in test_op_roi_align in test_operator

b9e15a6

marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed CI Flaky pr-awaiting-review PR is waiting for code review labels Jan 14, 2019

wkcn added 2 commits January 15, 2019 09:31

Merge branch 'master' into fix_roi_align_test

7079f9a

merge master

521eb5c

marcoabreu added pr-work-in-progress PR is still work in progress and removed pr-awaiting-merge Review and CI is complete. Ready to Merge labels Jan 15, 2019

wkcn added 2 commits January 16, 2019 21:21

merge master

6ab9233

Update test_operator.py

0c7dbe4

wkcn added 2 commits January 17, 2019 21:37

retrigger CI

072b208

Merge branch 'fix_roi_align_test' of github.com:wkcn/incubator-mxnet …

8698ae8

…into fix_roi_align_test

sandeep-krishnamurthy added Test pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress labels Jan 28, 2019

anirudhacharya approved these changes Jan 29, 2019

View reviewed changes

vandanavk approved these changes Feb 5, 2019

View reviewed changes

marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-review PR is waiting for code review labels Feb 5, 2019

sandeep-krishnamurthy merged commit 7c7af3a into apache:master Feb 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-1258]fix unittest for ROIAlign Operator #13609

[MXNET-1258]fix unittest for ROIAlign Operator #13609

wkcn commented Dec 11, 2018 •

edited

Loading

wkcn commented Dec 11, 2018 •

edited

Loading

roywei commented Dec 12, 2018

anirudhacharya commented Dec 17, 2018

wkcn commented Dec 17, 2018 •

edited

Loading

anirudhacharya left a comment

anirudhacharya Dec 19, 2018

wkcn Dec 19, 2018

anirudhacharya Dec 19, 2018

wkcn Dec 19, 2018

Roshrini commented Jan 2, 2019

anirudhacharya commented Jan 2, 2019

anirudhacharya commented Jan 14, 2019

wkcn commented Jan 15, 2019 •

edited

Loading

stu1130 commented Jan 15, 2019 •

edited

Loading

wkcn commented Jan 16, 2019

wkcn commented Jan 17, 2019 •

edited

Loading

wkcn commented Jan 17, 2019

sandeep-krishnamurthy commented Jan 28, 2019

anirudhacharya commented Jan 29, 2019

vandanavk left a comment

vandanavk commented Feb 5, 2019

[MXNET-1258]fix unittest for ROIAlign Operator #13609

[MXNET-1258]fix unittest for ROIAlign Operator #13609

Conversation

wkcn commented Dec 11, 2018 • edited Loading

Description

Checklist

Essentials

Changes

Comments

wkcn commented Dec 11, 2018 • edited Loading

roywei commented Dec 12, 2018

anirudhacharya commented Dec 17, 2018

wkcn commented Dec 17, 2018 • edited Loading

anirudhacharya left a comment

Choose a reason for hiding this comment

anirudhacharya Dec 19, 2018

Choose a reason for hiding this comment

wkcn Dec 19, 2018

Choose a reason for hiding this comment

anirudhacharya Dec 19, 2018

Choose a reason for hiding this comment

wkcn Dec 19, 2018

Choose a reason for hiding this comment

Roshrini commented Jan 2, 2019

anirudhacharya commented Jan 2, 2019

anirudhacharya commented Jan 14, 2019

wkcn commented Jan 15, 2019 • edited Loading

stu1130 commented Jan 15, 2019 • edited Loading

wkcn commented Jan 16, 2019

wkcn commented Jan 17, 2019 • edited Loading

wkcn commented Jan 17, 2019

sandeep-krishnamurthy commented Jan 28, 2019

anirudhacharya commented Jan 29, 2019

vandanavk left a comment

Choose a reason for hiding this comment

vandanavk commented Feb 5, 2019

wkcn commented Dec 11, 2018 •

edited

Loading

wkcn commented Dec 11, 2018 •

edited

Loading

wkcn commented Dec 17, 2018 •

edited

Loading

wkcn commented Jan 15, 2019 •

edited

Loading

stu1130 commented Jan 15, 2019 •

edited

Loading

wkcn commented Jan 17, 2019 •

edited

Loading