Skip to content

Make storage access error NotImplementedError#53972

Closed
ezyang wants to merge 6 commits intogh/ezyang/944/basefrom
gh/ezyang/944/head
Closed

Make storage access error NotImplementedError#53972
ezyang wants to merge 6 commits intogh/ezyang/944/basefrom
gh/ezyang/944/head

Conversation

@ezyang
Copy link
Copy Markdown
Contributor

@ezyang ezyang commented Mar 14, 2021

Stack from ghstack:

Signed-off-by: Edward Z. Yang ezyang@fb.com

Differential Revision: D27036573

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Mar 14, 2021

💊 CI failures summary and remediations

As of commit ec277ac (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 to the (internal) Dr. CI Users group.

ezyang added 2 commits March 14, 2021 08:30
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
@ezyang ezyang requested review from albanD and bhosmer March 15, 2021 15:46
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
ezyang added 2 commits March 15, 2021 11:23
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
Comment thread c10/core/TensorImpl.cpp

void TensorImpl::throw_storage_access_error() const {
TORCH_CHECK(false, "Cannot access storage of ", tensorimpl_type_name());
TORCH_CHECK_NOT_IMPLEMENTED(false, "Cannot access storage of ", tensorimpl_type_name());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well this one is not strictly a not implemented error? Maybe more of a TypeError?

But if that makes the whole Meta testing simpler, I understand :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this PR is kind of wrong, but it is "easier"; because I then need to go fix the test suite to stop assuming that there is a storage(). If you like I can remove this from the stack and maintain this in my slush patch until I get far enough that I don't need it anymore.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok I think it is ok if it makes things easier.

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ezyang merged this pull request in 4878415.

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/944/head branch March 20, 2021 14:15
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Pull Request resolved: pytorch#53972

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: Imported from OSS

Reviewed By: mruberry

Differential Revision: D27036573

Pulled By: ezyang

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants