Skip to content

Conversation

@smessmer
Copy link
Contributor

@smessmer smessmer commented Oct 17, 2019

Stack from ghstack:

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

Differential Revision: D17969014

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
@ezyang
Copy link
Contributor

ezyang commented Oct 17, 2019

I'm pretty sure I don't want ConstQuantizerPtr permanently in the list. @jerryzh168 you have previously told me this name is temporary. What is the plan?

At the VERY least the types we aren't going to stick to should be prominently documented as such.

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
@smessmer
Copy link
Contributor Author

I added a bunch of TODOs to these types

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
Copy link
Contributor

@zdevito zdevito 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 except for DimnameList.

@jerryzh168
Copy link
Contributor

I'm pretty sure I don't want ConstQuantizerPtr permanently in the list. @jerryzh168 you have previously told me this name is temporary. What is the plan?

At the VERY least the types we aren't going to stick to should be prominently documented as such.

we'll have a Quantizer type that will replace this one in the future, and it's blocked on TorchBind supporting custom times right now, @jamesr66a has signed up to work on it.

{"QScheme", IntType::get()},
{"Storage", IntType::get()},
{"QScheme", IntType::get()}, // TODO This type should be removed from the schema parser, it should use the custom class mechanism instead. @jerryzh
{"ConstQuantizerPtr", IntType::get()}, // TODO This type should be removed from the schema parser, it should use the custom class mechanism instead. @jerryzh
Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, I'm fine landing it as is right now though

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.

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

[ghstack-poisoned]
zdevito pushed a commit to zdevito/ATen that referenced this pull request Oct 23, 2019
Summary:
Pull Request resolved: pytorch/pytorch#28181

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.
ghstack-source-id: 92436989

Test Plan: waitforsandcastle

Differential Revision: D17969014

fbshipit-source-id: 41ebe256baec81ed8fb165e7b7cffa5160d285c3
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a94bf1d.

@facebook-github-bot facebook-github-bot deleted the gh/smessmer/83/head branch October 28, 2019 22:20
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
Summary:
Pull Request resolved: pytorch#28181

These types are needed to parse the schemas from native_functions.yaml.

Note: This doesn't actually add the functionality to JIT, it only makes the parser pass.
ghstack-source-id: 92436989

Test Plan: waitforsandcastle

Differential Revision: D17969014

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

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants