Skip to content

Refactor type conversion functions out of //torch_xla/csrc:tensor#5777

Merged
will-cromar merged 9 commits intomasterfrom
wcromar/refactor-dtype
Nov 9, 2023
Merged

Refactor type conversion functions out of //torch_xla/csrc:tensor#5777
will-cromar merged 9 commits intomasterfrom
wcromar/refactor-dtype

Conversation

@will-cromar
Copy link
Copy Markdown
Collaborator

This helps prevent circular dependencies. See #5772 for motivation.

  • Create dtype.cpp/h for type conversions.
  • Attempt to make naming more clear and separate "pure" conversions from those that upcast/downcast based on the device.
  • Consolidate some giant switch statements.

@will-cromar will-cromar requested a review from JackCaoG November 8, 2023 01:22
@will-cromar will-cromar marked this pull request as ready for review November 8, 2023 01:22
Comment thread test/cpp/cpp_test_util.cpp Outdated
for (auto& literal : literals) {
tensors.push_back(MakeTensorFromXlaLiteral(
literal, TensorTypeFromXlaType(literal.shape().element_type())));
literal, MaybeUpcastForHost(literal.shape().element_type())));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I understand the motivation of this renaming but it made it less obvious to see this function does a conversation from XLA type to Tensor type. MaybeUpcastXlaTypeToTensorType? Same for the downcast.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Renamed to MaybeUpcastToHostTorchType and MaybeDowncastToXlaDeviceType to clarify the return type.

@will-cromar will-cromar merged commit ee210bb into master Nov 9, 2023
mbzomowski pushed a commit to mbzomowski-test-org/xla that referenced this pull request Nov 16, 2023
…ytorch#5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
zpcore pushed a commit that referenced this pull request Nov 21, 2023
…5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
ManfeiBai pushed a commit that referenced this pull request Nov 29, 2023
…5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
ManfeiBai pushed a commit that referenced this pull request Nov 29, 2023
…5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
chunnienc pushed a commit to chunnienc/xla that referenced this pull request Dec 14, 2023
…ytorch#5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
golechwierowicz pushed a commit that referenced this pull request Jan 12, 2024
…5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

* remove extra line
bhavya01 pushed a commit that referenced this pull request Apr 22, 2024
…5777)

* Move pure dtype conversion functions to `dtype.cpp`

* remove comments

* better names

* fix includes

* formatting

* consolidate

* fix test build

* more explicit names

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants