Skip to content

Conversation

ffuuugor
Copy link
Contributor

Conv3d grad sample test is failing currently: CircleCI

This is most likely due to a difference in unfold3d method between opacus and EW implementations.

Opacus:

if dilation != (1, 1, 1):
    tensor = filter_dilated_rows(tensor, dilation, dilated_kernel_size, kernel_size)

EW:

if dilation != (1, 1, 1):
    raise NotImplementedError(f"dilation={dilation} not supported.")

As dilation other than 1 is not supported under any conditions with EW, I've fixed thenis_ew_compatible flag.

PS: @ashkan-software do we have a github issue to update EW's unfold3d with the recent updates you made?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 21, 2022
@ffuuugor ffuuugor changed the title Conv3d Conv3d test fix Oct 21, 2022
Copy link
Contributor

@alexandresablayrolles alexandresablayrolles left a comment

Choose a reason for hiding this comment

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

LGTM

@facebook-github-bot
Copy link
Contributor

@ffuuugor has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ffuuugor has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@ffuuugor has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@karthikprasad karthikprasad deleted the ffuuugor_testfix branch November 4, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants