Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

fix flasky unittest for deformable psroi pooling #12178

Merged
merged 2 commits into from
Aug 17, 2018
Merged

fix flasky unittest for deformable psroi pooling #12178

merged 2 commits into from
Aug 17, 2018

Conversation

HaozhiQi
Copy link
Contributor

Description

This PR aims to fix the problem raised by issue #11713.

Checklist

Essentials

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

The deformable ps roi pooling operator includes several corner cases which the function is not differentiable. Therefore during the unittest, we need to add some check to ensure the input values are within the smooth regions.

Comments

The issue and solution are discussed with @ankkhedia, @sandeep-krishnamurthy, @YuwenXiong and the fix is ran by @ankkhedia for about 2500 times on his local machine.

@ankkhedia could you help review it?

# By now we only have gpu implementation
if default_context().device_type == 'gpu':
check_numeric_gradient(op, [im_data, rois_data, offset_data], rtol=rtol, atol=atol,
grad_nodes=grad_nodes, ctx=mx.gpu(0))
grad_nodes=grad_nodes, ctx=mx.gpu(1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, this is not intentional. I fixed it already.

@ankkhedia
Copy link
Contributor

The changes looks good to me and have verified it locally for 2500 runs with random seeds.

@sandeep-krishnamurthy sandeep-krishnamurthy added Flaky pr-awaiting-merge Review and CI is complete. Ready to Merge labels Aug 15, 2018
Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Thank you @oh233
LGTM.

@sandeep-krishnamurthy
Copy link
Contributor

@marcoabreu - Your concerns are addressed. This is ready to be merged.

@ankkhedia
Copy link
Contributor

@marcoabreu Your concerns are addressed. Could you please merge this PR?

@marcoabreu marcoabreu merged commit 2b4d512 into apache:master Aug 17, 2018
marcoabreu added a commit that referenced this pull request Aug 17, 2018
piyushghai added a commit to piyushghai/incubator-mxnet that referenced this pull request Aug 17, 2018
@marcoabreu
Copy link
Contributor

Everyone, please check #11713

XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
* fix flasky unittest for deformable psroi pooling

* Fix incorrect gpu specification
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Flaky pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants