- Notifications
You must be signed in to change notification settings - Fork26.3k
Add torch.can_cast(from, to) function#26805
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nairbv commentedSep 25, 2019
@pytorchbot rebase this please |
zou3519 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Are there tests forat::canCast somewhere?
| -func:result_type.Scalar_Scalar(Scalar scalar1, Scalar scalar2) -> ScalarType | ||
| -func:can_cast(ScalarType from, ScalarType to) -> bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is removingvariants: function from result_type. However that's the default so it doesn't matter, but I am mentioning that just in case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ah didn't notice, that probably happened when I fixed my merge conflict with myself. Interesting that it's the default too.
| deftest_can_cast(self): | ||
| self.assertTrue(torch.can_cast(torch.double,torch.float)) | ||
| self.assertFalse(torch.can_cast(torch.float,torch.int)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you have unit tests forcanCast (the cpp function) somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't, do you think it'd be helpful? it's called/used pretty heavily in the actual arithmetic operators and in tensoriterator, which does have a lot of python test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm ideally we'd unit test heavily-used helper functions likecanCast during development instead of testing them indirectly by testing arithmetic operators so that we can be sure that we're building on top of working code. But since we have all of those tests for arithmetic operators and type promotion this should be fine.
facebook-github-bot left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nairbv is landing this pull request. If you are a Facebook employee, you can view this diffon Phabricator.
Summary:pytorch/pytorch#25472Pull Requestresolved:pytorch/pytorch#26805Differential Revision: D17628434Pulled By: nairbvfbshipit-source-id: 6af8031ac3afda1505d338075c0637ad043f8b7e
facebook-github-bot commentedSep 27, 2019
Summary:pytorch#25472Pull Requestresolved:pytorch#26805Differential Revision: D17628434Pulled By: nairbvfbshipit-source-id: 6af8031ac3afda1505d338075c0637ad043f8b7e
#25472