Skip to content

Conversation

@lkdvos
Copy link
Member

@lkdvos lkdvos commented Mar 16, 2025

This PR is part of a large set of changes that alters the backend from TensorKit.MatrixAlgebra to MatrixAlgebraKit for the factorizations.
Concretely, this is achieved through a new submodule TensorKit.Factorizations that implements most of the MatrixAlgebraKit interface, along with some overloads and reimplementations to be somewhat backwards compatible.

New features

  • foreachblock(f, t::AbstractTensorMap...) is a new function that is meant to provide a uniform interface to iterate through the blocks of a collection of tensors. The main purpose is to centralize this concept to easily add multithreading over the blocks. In order to break up this PR the actual multithreading implementation is left for a follow-up PR.
  • Truncated eigenvalue decompositions through eig_trunc and eigh_trunc
  • The factorizations can now swap out their backends to select alternate algorithms or implementations.
  • ominus and its unicode variant can now be used to obtain the orthogonal complement of a space, ie if W = oplus(V1, V2), V2 = ominus(W, V1).

Breaking changes

  • The OrthogonalFactorization structs have been removed, but their constructors are deprecated to obtain equivalent MatrixAlgebraKit algorithm structs.
  • The factorization functions leftorth, rightorth, tsvd, eig, eigh have been deprecated in favor of the MatrixAlgebraKit variants.
  • The truncation strategies are now directly inherited from MatrixAlgebraKit. This means truncdim is replaced by truncrank, trunctol is replaced by truncbelow, and there are deprecation warnings for the old functions.
  • The left_orth and right_orth functions will now always output tensors with a single space connecting them, which was not the case for Polar decompositions previously. To retrieve the old behavior with isposdef R factors (equal domain and codomain instead of isomorphic), the functions left_polar and right_polar can be used.
  • The left_orth and right_orth functions will now always have a connecting space with isdual=false. This is different from the previous behavior where some N,1 or 1,N tensors kept the isdual flag on the connecting leg.
  • There is no more direct call to both permute and factorize in one go. This is mainly because this is incompatible with the distinction between permute and braid.

@lkdvos lkdvos requested a review from Jutho March 16, 2025 20:19
@lkdvos lkdvos marked this pull request as draft March 16, 2025 20:19
@codecov
Copy link

codecov bot commented Mar 16, 2025

Codecov Report

❌ Patch coverage is 64.38547% with 255 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.02%. Comparing base (e852aa3) to head (8549b2a).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/tensors/factorizations/matrixalgebrakit.jl 61.37% 112 Missing ⚠️
src/tensors/factorizations/truncation.jl 53.33% 70 Missing ⚠️
src/tensors/factorizations/factorizations.jl 42.35% 49 Missing ⚠️
src/tensors/factorizations/implementations.jl 90.42% 9 Missing ⚠️
src/tensors/backends.jl 0.00% 8 Missing ⚠️
ext/TensorKitChainRulesCoreExt/factorizations.jl 92.10% 3 Missing ⚠️
src/spaces/cartesianspace.jl 0.00% 3 Missing ⚠️
src/spaces/vectorspaces.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
- Coverage   82.85%   78.02%   -4.84%     
==========================================
  Files          44       49       +5     
  Lines        5757     5760       +3     
==========================================
- Hits         4770     4494     -276     
- Misses        987     1266     +279     

☔ 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.

@lkdvos
Copy link
Member Author

lkdvos commented Jun 12, 2025

The test failures on empty tensors should be resolved once QuantumKitHub/MatrixAlgebraKit.jl#37 is merged.

@Jutho I think this is ready for another round of review, it definitely requires a bunch more cleanup but I think I am getting somewhere now, the improvements in MatrixAlgebraKit have definitely helped a bunch.
I'm struggling quite a bit with figuring out how breaking of a change I would be okay with, but making sure all old algorithms and syntaxes still work is turning out to be a major pain.
I'm not so sure if it is even worth it to keep QR, SVD, ... around, and while I could make them type aliases for their MatrixAlgebraKit counterparts in the meantime, this doesn't work for QRpos. I am leaning towards just doing that though, since it would definitely simplify more code.

@lkdvos lkdvos force-pushed the matrixalgebra branch 4 times, most recently from dc34c24 to 1903b01 Compare June 16, 2025 15:15
@lkdvos lkdvos marked this pull request as ready for review June 17, 2025 01:42
@lkdvos lkdvos enabled auto-merge (squash) October 2, 2025 15:54
@lkdvos lkdvos disabled auto-merge October 3, 2025 12:18
@lkdvos lkdvos merged commit a7d2bbb into main Oct 3, 2025
10 of 11 checks passed
@lkdvos lkdvos deleted the matrixalgebra branch October 3, 2025 12:19
Copilot AI mentioned this pull request Oct 3, 2025
5 tasks
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.

4 participants