Skip to content

Add torch.can_cast(from, to) function#26805

Closed
nairbv wants to merge 3 commits intopytorch:masterfrom
nairbv:can_cast
Closed

Add torch.can_cast(from, to) function#26805
nairbv wants to merge 3 commits intopytorch:masterfrom
nairbv:can_cast

Conversation

@nairbv
Copy link
Copy Markdown
Collaborator

@nairbv nairbv commented Sep 25, 2019

@pytorchbot pytorchbot added module: docs Related to our documentation, both in docs/ and docblocks module: internals Related to internal abstractions in c10 and ATen module: operators labels Sep 25, 2019
@nairbv nairbv added this to the 1.3 milestone Sep 25, 2019
@nairbv nairbv requested a review from gchanan September 25, 2019 14:45
@nairbv nairbv mentioned this pull request Sep 25, 2019
@nairbv
Copy link
Copy Markdown
Collaborator Author

nairbv commented Sep 25, 2019

@pytorchbot rebase this please

@nairbv nairbv requested a review from zou3519 September 26, 2019 19:56
Copy link
Copy Markdown
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

Are there tests for at::canCast somewhere?


- func: result_type.Scalar_Scalar(Scalar scalar1, Scalar scalar2) -> ScalarType

- func: can_cast(ScalarType from, ScalarType to) -> bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is removing variants: function from result_type. However that's the default so it doesn't matter, but I am mentioning that just in case

Copy link
Copy Markdown
Collaborator Author

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.


def test_can_cast(self):
self.assertTrue(torch.can_cast(torch.double, torch.float))
self.assertFalse(torch.can_cast(torch.float, torch.int))
Copy link
Copy Markdown
Contributor

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 for canCast (the cpp function) somewhere?

Copy link
Copy Markdown
Collaborator Author

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.

Copy link
Copy Markdown
Contributor

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 like canCast 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.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a 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 diff on Phabricator.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Sep 27, 2019
Summary:
pytorch/pytorch#25472
Pull Request resolved: pytorch/pytorch#26805

Differential Revision: D17628434

Pulled By: nairbv

fbshipit-source-id: 6af8031ac3afda1505d338075c0637ad043f8b7e
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@nairbv merged this pull request in 2a43b74.

pdlive215 pushed a commit to pdlive215/pytorch that referenced this pull request Nov 27, 2019
Summary:
pytorch#25472
Pull Request resolved: pytorch#26805

Differential Revision: D17628434

Pulled By: nairbv

fbshipit-source-id: 6af8031ac3afda1505d338075c0637ad043f8b7e
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
pytorch#25472
Pull Request resolved: pytorch#26805

Differential Revision: D17628434

Pulled By: nairbv

fbshipit-source-id: 6af8031ac3afda1505d338075c0637ad043f8b7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: docs Related to our documentation, both in docs/ and docblocks module: internals Related to internal abstractions in c10 and ATen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants