Skip to content

Conversation

@gchanan
Copy link
Contributor

@gchanan gchanan commented Aug 24, 2020

Stack from ghstack:

Differential Revision: D23306786

@gchanan gchanan requested a review from apaszke as a code owner August 24, 2020 23:04
gchanan added a commit that referenced this pull request Aug 24, 2020
…_loss, and mse_loss.

ghstack-source-id: 146614e
Pull Request resolved: #43527
@gchanan gchanan changed the title Properly check that reduction strings are valid for l1_loss, smoothl1_loss, and mse_loss. [WIP] Properly check that reduction strings are valid for l1_loss, smoothl1_loss, and mse_loss. Aug 24, 2020
@gchanan
Copy link
Contributor Author

gchanan commented Aug 24, 2020

TODO: testing

@gchanan gchanan added the module: bc-breaking Related to a BC-breaking change label Aug 24, 2020
@dr-ci
Copy link

dr-ci bot commented Aug 25, 2020

💊 CI failures summary and remediations

As of commit 24b31ac (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 or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 10 times.

gchanan added a commit that referenced this pull request Aug 27, 2020
…_loss, and mse_loss.

ghstack-source-id: 7842029
Pull Request resolved: #43527
…l1_loss, smoothl1_loss, and mse_loss."

Differential Revision: [D23306786](https://our.internmc.facebook.com/intern/diff/D23306786)

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Aug 27, 2020
…_loss, and mse_loss.

ghstack-source-id: 9751dfa
Pull Request resolved: #43527
@gchanan gchanan changed the title [WIP] Properly check that reduction strings are valid for l1_loss, smoothl1_loss, and mse_loss. Properly check that reduction strings are valid for l1_loss, smoothl1_loss, and mse_loss. Aug 27, 2020
@gchanan gchanan requested a review from albanD August 27, 2020 22:57
@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

Merging #43527 into gh/gchanan/302/base will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                   Coverage Diff                   @@
##           gh/gchanan/302/base   #43527      +/-   ##
=======================================================
+ Coverage                69.33%   69.34%   +0.01%     
=======================================================
  Files                      378      378              
  Lines                    46741    46744       +3     
=======================================================
+ Hits                     32406    32414       +8     
+ Misses                   14335    14330       -5     
Impacted Files Coverage Δ
torch/nn/functional.py 92.22% <100.00%> (+0.18%) ⬆️
torch/testing/_internal/expecttest.py 78.57% <0.00%> (+1.02%) ⬆️
torch/nn/_reduction.py 83.87% <0.00%> (+6.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86199dc...24b31ac. Read the comment docs.

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.

LGTM

F.ctc_loss() is not tested here, is that on purpose?

@gchanan
Copy link
Contributor Author

gchanan commented Aug 31, 2020

good point, I'll add ctc_loss.

@facebook-github-bot
Copy link
Contributor

@gchanan merged this pull request in 42c895d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: bc-breaking Related to a BC-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants