Skip to content

implemented Trace for GPU#749

Open
manuschneider wants to merge 4 commits intomasterfrom
trace
Open

implemented Trace for GPU#749
manuschneider wants to merge 4 commits intomasterfrom
trace

Conversation

@manuschneider
Copy link
Copy Markdown
Collaborator

@manuschneider manuschneider commented Mar 18, 2026

Currently, Tensor.Trace corresponds to creating a UniTensor and contracting it with a (diagonal) identity UniTensor;
removed Nomp argument in internal Trace functions;

This fixes #735

…ating a UniTensor and contracting it with a (diagonal) identity UniTensor
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

❌ Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.53%. Comparing base (7aa7862) to head (f02a7a8).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
src/linalg/Trace.cpp 22.22% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #749   +/-   ##
=======================================
  Coverage   35.53%   35.53%           
=======================================
  Files         214      214           
  Lines       33016    33018    +2     
  Branches    13133    13133           
=======================================
+ Hits        11731    11732    +1     
- Misses      19361    19362    +1     
  Partials     1924     1924           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@manuschneider manuschneider added the Pending check/approval Issue fixed, and need feedback label Mar 18, 2026
Comment on lines +17 to +22
cytnx::UniTensor I_UT = cytnx::UniTensor::eye(Ndiag, {}, true, Tn.dtype(), Tn.device());
// similar to _trace_nd_gpu
UniTensor UTn = UniTensor(Tn, false, 2);
I_UT.set_labels({UTn._impl->_labels[0], UTn._impl->_labels[1]});

out = Contract(I_UT, UTn).get_block_();
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.

Is this less efficient than the original implementation?

@ianmccul
Copy link
Copy Markdown
Collaborator

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 736a1666b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

UniTensor UTn = UniTensor(Tn, false, 2);
I_UT.set_labels({UTn._impl->_labels[0], UTn._impl->_labels[1]});

out = Contract(I_UT, UTn).get_block_();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Handle rectangular matrix trace on GPU

Tensor::Trace computes Ndiag = min(dim[axisA], dim[axisB]), so rectangular traces are expected to work, but this GPU path now contracts eye(Ndiag) against the full 2D tensor. For inputs like shape (m,n) with m != n, one contracted label has size Ndiag while the tensor bond still has size max(m,n), so Contract sees mismatched bond dimensions and raises instead of returning the diagonal sum (CPU still handles this case).

Useful? React with 👍 / 👎.

@manuschneider manuschneider removed the Pending check/approval Issue fixed, and need feedback label Mar 30, 2026
…nt number of elements is currently not implemented
@manuschneider
Copy link
Copy Markdown
Collaborator Author

I do not think that tracing over indices with different dimensions is an essential feature that we need to support at the moment.
For now, I added error messages when the user tries this, but the backend does not support it.

In the long term, it would be good to implement more efficient ways to calculate the trace on both CPU and GPU without multiplying by the identity. Then, we can also decide whether to allow traces over indices of different lengths. This is not a high priority currently, though.

@manuschneider manuschneider added the Pending check/approval Issue fixed, and need feedback label Apr 9, 2026
@ianmccul
Copy link
Copy Markdown
Collaborator

ianmccul commented Apr 9, 2026

I think there is no reason to ever allow trace over a non-square tensor. By definition that means that the left right indices represent different spaces, so the trace is meaningless. In the special case where one space is a diagonal projection $P$ of the other space, it might make sense to calculate something like $\text{Tr} PM$ and optimize that if necessary.

@manuschneider
Copy link
Copy Markdown
Collaborator Author

An error message is now thrown in all cases where the index dimensions of the traced indices do not match.

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

Labels

Pending check/approval Issue fixed, and need feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing GPU implementation for Trace

3 participants