Add type annotation logic for complex numbers by anjali411 · Pull Request #50884 · pytorch/pytorch

anjali411 · 2021-01-21T15:50:35Z

Stack from ghstack:

Support complex number list in JIT #51145 Support complex number list in JIT
Add serialization logic for complex numbers #50885 Add serialization logic for complex numbers
Add type annotation logic for complex numbers #50884 Add type annotation logic for complex numbers

Differential Revision:

Differential Revision: D26086963

[ghstack-poisoned]

facebook-github-bot · 2021-01-21T17:19:49Z

💊 CI failures summary and remediations

As of commit 33e9e2a (more details on the Dr. CI page):

1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_linux_xenial_py3_6_gcc5_4_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jan 26 22:48:53 AssertionError: mypy failed: torch/fx/experimental/accelerated_graph_module.py:4: error: Cannot find implementation or library stub for module named 'glow.fb.fx_glow_binding.fx_glow'  [import]

Jan 26 22:47:34 Test results will be stored in test-reports/python-unittest
Jan 26 22:47:42   test_doc_examples (__main__.TestTypeHints) ... ok (7.820s)
Jan 26 22:48:53   test_run_mypy (__main__.TestTypeHints) ... FAIL (71.291s)
Jan 26 22:48:53 
Jan 26 22:48:53 ======================================================================
Jan 26 22:48:53 FAIL [71.291s]: test_run_mypy (__main__.TestTypeHints) [mypy.ini]
Jan 26 22:48:53 ----------------------------------------------------------------------
Jan 26 22:48:53 Traceback (most recent call last):
Jan 26 22:48:53   File "test_type_hints.py", line 171, in test_run_mypy
Jan 26 22:48:53     self.fail(f"mypy failed: {stdout} {stderr}")
Jan 26 22:48:53 AssertionError: mypy failed: torch/fx/experimental/accelerated_graph_module.py:4: error: Cannot find implementation or library stub for module named 'glow.fb.fx_glow_binding.fx_glow'  [import]
Jan 26 22:48:53 torch/fx/experimental/accelerated_graph_module.py:4: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
Jan 26 22:48:53 torch/fx/experimental/accelerated_graph_module.py:4: error: Cannot find implementation or library stub for module named 'glow'  [import]
Jan 26 22:48:53 torch/fx/experimental/accelerated_graph_module.py:4: error: Cannot find implementation or library stub for module named 'glow.fb'  [import]
Jan 26 22:48:53 torch/fx/experimental/accelerated_graph_module.py:4: error: Cannot find implementation or library stub for module named 'glow.fb.fx_glow_binding'  [import]
Jan 26 22:48:53 Found 4 errors in 1 file (checked 1212 source files)
Jan 26 22:48:53  
Jan 26 22:48:53 
Jan 26 22:48:53 ----------------------------------------------------------------------
Jan 26 22:48:53 Ran 2 tests in 79.111s
Jan 26 22:48:53

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.

[ghstack-poisoned]

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

SplitInfinity

Nice! Do you plan to support constant literals in TorchScript code in this diff? If not, I think it is ready to land. If so, I think some more changes are required.

SplitInfinity · 2021-01-26T19:50:37Z

        return py::cast<int64_t>(obj);
      } else if (py::isinstance<py::float_>(obj)) {
        return py::cast<double>(obj);
+      } else if (PyComplex_CheckExact(obj.ptr())) {


Dumb question but why does this line not say something like py::isinstance<py::complex_>(...) like all the others?

Complex type is not available in pybind (see doc here: https://pybind11.readthedocs.io/en/stable/advanced/pycpp/object.html#accessing-python-libraries-from-c) so I just pass the underlying PyObject* to PyComplex_CheckExact to check whether it's complex

SplitInfinity · 2021-01-26T19:55:11Z

+    if inspect.isclass(ann) and issubclass(ann, builtins.complex):
+        return ComplexDoubleType.get()


Another dumb question, why is this not ann is complex?

Also, is there a usecase for creating subclasses of complex?

complex is a python class hence the isclass call. umm no there's no use specific case of creating subclasses of complex

But aren't int and float classes too?

ok wait you are right ann is complex works!

SplitInfinity · 2021-01-26T19:59:09Z

+      case TypeKind::ComplexDoubleType:
        return parseSingleConstant(arg_type->kind());


parseSingleConstant needs to be updated to handle complex numbers. I would either get rid of it and do it in a follow-up PR or add a test for it (and update parseSingleConstant and other code as needed).

I'll just remove this change for the current PR and add it separately

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

anjali411 · 2021-01-26T22:04:24Z

Nice! Do you plan to support constant literals in TorchScript code in this diff? If not, I think it is ready to land. If so, I think some more changes are required.

no I'll just make a separate PR for that! also, addressed all the comments and updated the PR

SplitInfinity

🚀

facebook-github-bot · 2021-01-27T03:42:55Z

This pull request has been merged in b6eaca9.

Summary: Pull Request resolved: pytorch#50884 Test Plan: Imported from OSS Reviewed By: heitorschueroff Differential Revision: D26086963 fbshipit-source-id: f103f7f529d63d701c4f17862e30eafbab7d0c68

Add type annotation logic for complex numbers

72352ba

[ghstack-poisoned]

This was referenced Jan 21, 2021

Add complex IValues #50883

Closed

Add serialization logic for complex numbers #50885

Closed

facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels Jan 21, 2021

anjali411 added 5 commits January 21, 2021 09:52

Update on "Add type annotation logic for complex numbers"

706f5b5

[ghstack-poisoned]

Update on "Add type annotation logic for complex numbers"

bb73776

[ghstack-poisoned]

Update on "Add type annotation logic for complex numbers"

a24b346

[ghstack-poisoned]

Update on "Add type annotation logic for complex numbers"

0d68c4f

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

Update on "Add type annotation logic for complex numbers"

9c80a8f

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

anjali411 requested a review from SplitInfinity January 26, 2021 16:08

Update on "Add type annotation logic for complex numbers"

ba6674c

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

anjali411 added the module: complex Related to complex number support in PyTorch label Jan 26, 2021

SplitInfinity reviewed Jan 26, 2021

View reviewed changes

Update on "Add type annotation logic for complex numbers"

33e9e2a

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

anjali411 mentioned this pull request Jan 26, 2021

Support complex number list in JIT #51145

Closed

SplitInfinity approved these changes Jan 26, 2021

View reviewed changes

facebook-github-bot closed this in b6eaca9 Jan 27, 2021

facebook-github-bot added the Merged label Jan 27, 2021

facebook-github-bot deleted the gh/anjali411/88/head branch January 30, 2021 15:21

		if inspect.isclass(ann) and issubclass(ann, builtins.complex):
		return ComplexDoubleType.get()

		case TypeKind::ComplexDoubleType:
		return parseSingleConstant(arg_type->kind());

Conversation

anjali411 commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_xenial_py3_6_gcc5_4_test (1/1)

Uh oh!

SplitInfinity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anjali411 commented Jan 26, 2021

Uh oh!

SplitInfinity left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anjali411 commented Jan 21, 2021 •

edited

Loading

facebook-github-bot commented Jan 21, 2021 •

edited

Loading