Skip to content

Conversation

@nairbv
Copy link
Collaborator

@nairbv nairbv commented Jun 25, 2019

This is (mostly) the re-application of:
#21088

which was reverted due to an issue conflicting with changes in:
#22104

@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: cpp-extensions Related to torch.utils.cpp_extension module: internals Related to internal abstractions in c10 and ATen module: onnx Related to torch.onnx module: operators module: pybind Related to our Python bindings / interactions with other Python libraries module: tests Issues related to tests (not the torch.testing module) labels Jun 25, 2019
if (auto opt_rhs = rhs.cast<OptionalType>()) {
return getElementType()->isSubtypeOf(opt_rhs->getElementType());
return *getElementType() == *opt_rhs->getElementType();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these are the new changes added to (hopefully) fix the tests that failed due to #22104

Copy link
Collaborator

Choose a reason for hiding this comment

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

#22186 removed == operator overload since the OptionalType's parent type SingleElementType already contains the correct overload here. you could rebase on after that get merged and it should solve your problem.

Copy link
Collaborator

@wanchaol wanchaol left a comment

Choose a reason for hiding this comment

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

looks good, the jit_type changes is landing so please rebase and you can safely delete the jit_type changes here.

if (auto opt_rhs = rhs.cast<OptionalType>()) {
return getElementType()->isSubtypeOf(opt_rhs->getElementType());
return *getElementType() == *opt_rhs->getElementType();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

#22186 removed == operator overload since the OptionalType's parent type SingleElementType already contains the correct overload here. you could rebase on after that get merged and it should solve your problem.

@nairbv
Copy link
Collaborator Author

nairbv commented Jun 26, 2019

@pytorchbot rebase this please

@pytorchbot
Copy link
Collaborator

Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below:

git fetch origin master
git fetch [email protected]:nairbv/pytorch.git re_apply_opt_scalardtype
git checkout FETCH_HEAD
git merge origin/master
git push [email protected]:nairbv/pytorch.git HEAD:re_apply_opt_scalardtype

(To learn more about this bot, see Bot commands.)

@nairbv
Copy link
Collaborator Author

nairbv commented Jun 26, 2019

for reference, this is the change that fixes this PR:
#22186

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.

@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 Jun 26, 2019
Summary:
This is (mostly) the re-application of:
pytorch/pytorch#21088

which was reverted due to an issue conflicting with changes in:
pytorch/pytorch#22104
Pull Request resolved: pytorch/pytorch#22237

Differential Revision: D16012838

Pulled By: nairbv

fbshipit-source-id: 35f4a73c97ab68b4e2648aca96b2176f07b5a883
@facebook-github-bot
Copy link
Contributor

@nairbv merged this pull request in 7707dee.

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

Labels

Merged module: cpp-extensions Related to torch.utils.cpp_extension module: internals Related to internal abstractions in c10 and ATen module: onnx Related to torch.onnx module: pybind Related to our Python bindings / interactions with other Python libraries module: tests Issues related to tests (not the torch.testing module) oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants