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

[MXNET-687] Fix spatial transformer op #11806

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

anirudh2290
Copy link
Member

@anirudh2290 anirudh2290 commented Jul 18, 2018

Description

Spatial transformer op produced incorrect results with USE_CUDA=ON USE_CUDNN=OFF till now. This was due to separate threads writing to the same global memory locations causing undefined behavior.
Replaced all such writes with atomicAdd. I have also reenabled the disabled test: test_stn.
Removed assertion_error param for assert_raises_cudnn_disabled.

This fixes #11568

@tornadomeet @marcoabreu

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:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@anirudh2290 anirudh2290 force-pushed the spatial_transformer branch from d5a179d to a224fee Compare July 18, 2018 22:37
@tornadomeet
Copy link
Contributor

@anirudh2290 the change of spatial_transformer.cu LGTM. the other part @szha

Copy link
Member

@szha szha left a comment

Choose a reason for hiding this comment

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

LGTM. Looks like CI caught another flaky test, so I just triggered the build again to get past it.

How does the performance (speed) change?

@anirudh2290
Copy link
Member Author

I haven't run any performance tests. Having said that, adding atomicAdd will make it slower but it is required for correctness of the OP.

@anirudh2290 anirudh2290 merged commit 74d6692 into apache:master Jul 24, 2018
XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with spatial transformer op when cudnn disabled
3 participants