Skip to content

Add complex IValues#50883

Closed
anjali411 wants to merge 6 commits intogh/anjali411/87/basefrom
gh/anjali411/87/head
Closed

Add complex IValues#50883
anjali411 wants to merge 6 commits intogh/anjali411/87/basefrom
gh/anjali411/87/head

Conversation

@anjali411
Copy link
Copy Markdown
Contributor

@anjali411 anjali411 commented Jan 21, 2021

Stack from ghstack:

Differential Revision: D26003682

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

facebook-github-bot commented Jan 21, 2021

💊 CI failures summary and remediations

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


  • 4/4 failures possibly* introduced in this PR
    • 2/4 non-CircleCI failure(s)

2 failures not recognized by patterns:

Job Step Action
CircleCI pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test2 Run tests 🔁 rerun
CircleCI pytorch_windows_vs2019_py36_cuda10.1_test2 Unknown 🔁 rerun

Extra GitHub checks: 1 failed


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.

Copy link
Copy Markdown

@SplitInfinity SplitInfinity left a comment

Choose a reason for hiding this comment

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

Nice! After further discussion with the JIT team about the place of ComplexDoubleType in the type system, we concluded that it makes more sense for it to be a subtype of only Any, and not of NumberType. Other than that, the rest looks good!

Comment thread aten/src/ATen/core/jit_type.h
Comment thread aten/src/ATen/core/jit_type.h
Comment thread aten/src/ATen/core/jit_type.h Outdated
Comment thread aten/src/ATen/core/jit_type.h Outdated
anjali411 added a commit that referenced this pull request Jan 21, 2021
ghstack-source-id: a9c8c5a
Pull Request resolved: #50883
// Tuples should be structurally typed
auto type = TupleType::create({IntType::get(), TensorType::get(), FloatType::get()});
auto type2 = TupleType::create({IntType::get(), TensorType::get(), FloatType::get()});
auto type = TupleType::create({IntType::get(), TensorType::get(), FloatType::get(), ComplexDoubleType::get()});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are these changes for testing ComplexDoubleType::operator==?

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 and also indirectly testing ComplexDoubleType::get()

Copy link
Copy Markdown

@SplitInfinity SplitInfinity left a comment

Choose a reason for hiding this comment

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

Let's make sure to follow up on the correct place for ComplexDoubleType in the JIT type system and fix it if needed in the future.

Approval contingent on tests passing.

@anjali411
Copy link
Copy Markdown
Contributor Author

Nice! After further discussion with the JIT team about the place of ComplexDoubleType in the type system, we concluded that it makes more sense for it to be a subtype of only Any, and not of NumberType. Other than that, the rest looks good!

Synced offline: It probably makes sense for ComplexDoubleType to be a subtype of NumberType since we can have complex scalars and functions like torch.full, torch.index_fill etc. can take complex numbers as input.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@anjali411 merged this pull request in 9ac30d9.

@facebook-github-bot facebook-github-bot deleted the gh/anjali411/87/head branch January 26, 2021 15:21
@anjali411 anjali411 added the module: complex Related to complex number support in PyTorch label Jan 26, 2021
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary: Pull Request resolved: pytorch#50883

Test Plan: Imported from OSS

Reviewed By: ejguan

Differential Revision: D26003682

Pulled By: anjali411

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

Labels

cla signed Merged module: complex Related to complex number support in PyTorch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants