KEMBAR78
[easy] Enforce non-negativity of tensor construction by dzhulgakov · Pull Request #17077 · pytorch/pytorch · GitHub
Skip to content

Conversation

@dzhulgakov
Copy link
Collaborator

Apparently, before the only way we enforced it was size>=0 in alloc_cpu. So empty((5,-5)) would fail but empty((-5,-5)) would hang :)

Please suggest better place to enforce it if any.

Copy link
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.

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

@ezyang
Copy link
Contributor

ezyang commented Feb 14, 2019

We have a generic problem where there are a lot of places where we are missing "common sense" checks like this. Probably the only way to squash them all out is with THE POWER OF TYPES

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: check_size_nonnegative? This sounds really generic.

Copy link
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.

@dzhulgakov 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 Feb 21, 2019
Summary:
Apparently, before the only way we enforced it was size>=0 in alloc_cpu. So empty((5,-5)) would fail but empty((-5,-5)) would hang :)

Please suggest better place to enforce it if any.
Pull Request resolved: pytorch/pytorch#17077

Differential Revision: D14077930

Pulled By: dzhulgakov

fbshipit-source-id: 1120513300fd5448e06fa15c2d72f9b0ee5734e4
@ezyang ezyang added the merged label Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants