Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add slope == 0 case into standard leaky relu nn test #37559

Closed
wants to merge 2 commits into from

Conversation

glaringlee
Copy link
Contributor

@glaringlee glaringlee commented Apr 30, 2020

Stack from ghstack:

Differential Revision: D21319922

glaringlee pushed a commit that referenced this pull request Apr 30, 2020
ghstack-source-id: aee295322eeffe70d56eff37e1aabab1ddd502aa
Pull Request resolved: #37559
@dr-ci
Copy link

dr-ci bot commented Apr 30, 2020

💊 Build failures summary and remediations

As of commit 7ccc8ba (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 2 times.

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.
The new test looks good !

@@ -5635,11 +5635,6 @@ def test_leaky_relu_inplace_with_zero_or_neg_slope(self, device):
with self.assertRaisesRegex(RuntimeError, "call out-of-place version"):
b.backward(torch.ones(2, device=device))

a = torch.tensor([-2., 0., 2.], device=device, requires_grad=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is worth keeping!
We do rely on this subgradient being 0. Even though we most likely will never willingly change that. It is good to have a test that makes sure it does not happen by mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. I will make a separate test to test this.

glaringlee pushed a commit that referenced this pull request Apr 30, 2020
ghstack-source-id: d7b20a65968c320da2d4de1e76d1578bf06d4eea
Pull Request resolved: #37559
@glaringlee glaringlee requested a review from albanD April 30, 2020 17:04
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks!

@facebook-github-bot
Copy link
Contributor

@glaringlee merged this pull request in 0692804.

@glaringlee glaringlee deleted the gh/glaringlee/17/head branch May 1, 2020 20:33
@glaringlee glaringlee restored the gh/glaringlee/17/head branch May 1, 2020 20:33
@facebook-github-bot facebook-github-bot deleted the gh/glaringlee/17/head branch May 4, 2020 14:17
gchanan pushed a commit that referenced this pull request May 5, 2020
Summary: Pull Request resolved: #37559

Test Plan: Imported from OSS

Differential Revision: D21319922

Pulled By: glaringlee

fbshipit-source-id: 212ef8e9d0f0d55a312d282693cd5990e0376c6a
@gchanan gchanan added this to the 1.5.1 milestone May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants