FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Promote storagetypes by lkdvos · Pull Request #370 · QuantumKitHub/TensorKit.jl · GitHub

Promote storagetypes - #370

Merged
lkdvos merged 9 commits into
mainfrom
promote_storagetype
Feb 10, 2026
Merged

lkdvos merged 9 commits into
mainfrom
promote_storagetype

Conversation

lkdvos commented Feb 9, 2026

Copy link
Copy Markdown
Member

This is an attempt to properly tackle storagetypes, and to expand some methods to allow for less concrete types.

The main issue that sparked this is QuantumKitHub/MPSKit.jl#376, where the blocktensors of an MPO contain Union{BraidingTensor,TensorMap} as eltype, and therefore give issues when determining the storagetype.
I was aware of this issue, and had resolved it with an evil hack, however as with most evil hacks, this at some point breaks because of small changes.
In this case, the recent change to add a method in TensorKit storagetype(t) = storagetype(typeof(t)) seemed to have bypassed my evil hack (which was utter and complete type piracy), therefore leading to linked issue.

The context is that in a sparse blocktensormap of a given element type, I need to be able to instantiate a zero tensor of that given type, and I would really like to keep the option of not having fully specified types as element types, such as the union of braidingtensor and regular tensormaps.
Currently, similar(::Type{<:AbstractTensorMap}, space) is defined in the type domain, but this ends up calling storagetype, which is where the error comes from.

Therefore I changed the storagetype definition here to have the following fallback behavior:

  • For <:AbstractTensorMap, I simply fall back to the default storagetype that is used in e.g. zeros(space), given by similarstoragetype(scalartype(t)) = Vector{T}. This is consistent with the previous fallback definition for storagetype(BraidingTensor), which I therefore removed
  • For Union types, I try to promote the storagetypes of the components, similar to how contraction or addition would try to do that.

This latter promotion system already had most of its skeleton in place, but still needed the actual promotion rules etc, similar to how base/promotion.jl defines this in Julia Base.
I copied over some of that, such that now this should also be easily extensible to GPU and CPU mixes, where we may define conversion types appropriately.
I have not defined any promotion rules in this PR since I don't actually know what the best definition would be though.

I'd be glad to hear some suggestions as to how to test this, but I did run the MPSKit testsuite on top of this and this does seem to resolve the issue.

lkdvos requested review from Jutho and kshyatt and removed request for kshyatt February 9, 2026 19:35

codecov Bot commented Feb 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 34.09091% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/tensors/abstracttensor.jl 25.80% 23 Missing ⚠️
src/tensors/braidingtensor.jl 0.00% 6 Missing ⚠️
Files with missing lines Coverage Δ
src/tensors/diagonal.jl 92.07% <100.00%> (-0.08%) ⬇️
src/tensors/linalg.jl 82.59% <100.00%> (-0.06%) ⬇️
src/tensors/tensor.jl 85.39% <100.00%> (-0.09%) ⬇️
src/tensors/tensoroperations.jl 97.42% <100.00%> (-0.04%) ⬇️
src/tensors/braidingtensor.jl 68.29% <0.00%> (-2.78%) ⬇️
src/tensors/abstracttensor.jl 54.88% <25.80%> (-3.77%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kshyatt previously approved these changes Feb 10, 2026
Jutho previously approved these changes Feb 10, 2026

Jutho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I find it hard to reason about this code, but it looks ok as far as I can tell. And if it is modelled after number promotion, I assume the compiler will be fine with it.

lkdvos commented Feb 10, 2026

Copy link
Copy Markdown
Member Author

Ok, I will merge it, double-check that everything is fine after I remove the BlockTensorKit hack and MPSKit runs again, and then make a patch release proposal.

lkdvos dismissed stale reviews from Jutho and kshyatt via 6d9d48a February 10, 2026 15:53

lkdvos commented Feb 10, 2026

Copy link
Copy Markdown
Member Author

@kshyatt I added the promotion for more than 2 inputs as well, since I saw you needed this in this PR: QuantumKitHub/MPSKit.jl#375
I can confirm that locally everything runs, so will merge and tag this after that.

lkdvos enabled auto-merge (squash) February 10, 2026 15:57
lkdvos requested a review from kshyatt February 10, 2026 15:57
lkdvos disabled auto-merge February 10, 2026 17:43
lkdvos merged commit 80bd0bc into main Feb 10, 2026
41 of 42 checks passed
lkdvos deleted the promote_storagetype branch February 10, 2026 17:43
lkdvos mentioned this pull request Feb 10, 2026
lkdvos referenced this pull request Feb 10, 2026
* update changelog

* Bump v0.16.2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL