Skip to content

[jit] Allow 'Any' to appear as a type argument.#26572

Closed
zdevito wants to merge 5 commits intogh/zdevito/113/basefrom
gh/zdevito/113/head
Closed

[jit] Allow 'Any' to appear as a type argument.#26572
zdevito wants to merge 5 commits intogh/zdevito/113/basefrom
gh/zdevito/113/head

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Sep 20, 2019

Stack from ghstack:

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

Differential Revision: D17530643

Copy link
Member

@suo suo left a comment

Choose a reason for hiding this comment

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

ngnt

self.assertEqual(any_refinement(3, 4), 7)
self.assertEqual(any_refinement(3, "hi"), 0)

def test_any_in_class_fails(self):
Copy link
Member

Choose a reason for hiding this comment

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

Can we add tests that verify that Any is not allowed in Tuples, NamedTuples, or Interfaces as well?

// static types in named types to reconstruct type tags of loaded
// values. Lifting this restriction requires solving the serialization
// problem first.
CAFFE2_API void checkNoAny(
Copy link
Member

Choose a reason for hiding this comment

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

I am a little wary of having this enforcement as opt-in. For example, if someone goes in and adds attributes to interfaces and forgets to include this check, we've very quickly produced the bug this comment describes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The specific example of interfaces will not actually cause this bug, but I agree in spirit. But how would we make it more likely to be called? There isn't a more common place to put it.

[jit] Allow 'Any'  to appear as a type argument.

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

gh-metadata: pytorch pytorch 26572 gh/zdevito/113/head
[jit] Allow 'Any'  to appear as a type argument.

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

gh-metadata: pytorch pytorch 26572 gh/zdevito/113/head
[jit] Allow 'Any'  to appear as a type argument.

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

gh-metadata: pytorch pytorch 26572 gh/zdevito/113/head
[jit] Allow 'Any'  to appear as a type argument.

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

gh-metadata: pytorch pytorch 26572 gh/zdevito/113/head
[jit] Allow 'Any'  to appear as a type argument.

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

gh-metadata: pytorch pytorch 26572 gh/zdevito/113/head
zdevito added a commit to zdevito/ATen that referenced this pull request Oct 16, 2019
Summary:
Pull Request resolved: pytorch/pytorch#26572

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

Test Plan: Imported from OSS

Differential Revision: D17530643

Pulled By: zdevito

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

@zdevito merged this pull request in fb45171.

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

Combined with isinstance specialization this allows a degree of polymorphic
functions to work without needing to use our weirder overload hacks.

We do not define any operators on Any, so the only thing you can do with it
is to put it in containers or type refine it using an isinstance check.
Any is restricted from appearing in non-argument position because we
cannot restore type tags if it ends up as a field in a class.

Test Plan: Imported from OSS

Differential Revision: D17530643

Pulled By: zdevito

fbshipit-source-id: f06f78ce84819f7773953a492f3d4c49219ee94c
inline Stack toTraceableStack(const py::tuple& inputs) {
auto info = toTypeInferredIValue(inputs);
AT_CHECK(
isTraceableType(info.type()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @zdevito
May I know why we only support Tensors and (possibly nested) Lists, Dicts, and Tuples of Tensors?
What if we passed a None type of python, and what will happen that makes us not support it?

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

Labels

Merged module: cpp Related to C++ API module: internals Related to internal abstractions in c10 and ATen module: pybind Related to our Python bindings / interactions with other Python libraries oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants