Skip to content

Conversation

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 31, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/166694

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ You can merge normally! (4 Unrelated Failures)

As of commit 7b6869c with merge base 4295a9a (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@mikaylagawarecki mikaylagawarecki marked this pull request as ready for review October 31, 2025 15:02
Device device() const {
int32_t device_type;
int32_t device_index;
TORCH_ERROR_CODE_CHECK(
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative to this implementation is adding a new shim function that returns a DeviceHandle, and then converts the DeviceHandle to the device. My understanding is that this current impl returns a new Device object.

  /// Returns a `Tensor`'s device.
  inline Device device() const {
    return impl_->device();
  }

This implementation in TensorBase.h suggests that we should be adding a new shim instead to return the same instance. What do you think?

Copy link
Contributor Author

@mikaylagawarecki mikaylagawarecki Oct 31, 2025

Choose a reason for hiding this comment

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

Is my understanding correct that that would mean the Tensor within libtorch owns the device object being used by the extension?

When we pass this across the libtorch/extension boundary, might the device need to outlive the tensor object in libtorch and would returning the DeviceHandle cause any issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think passing it through the boundary should change lifetimes if we're just rerouting pointers.

@mikaylagawarecki mikaylagawarecki added release notes: cpp release notes category topic: new features topic category labels Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: cpp release notes category topic: new features topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants