Skip to content

Conversation

lkdvos
Copy link
Contributor

@lkdvos lkdvos commented Dec 12, 2024

This PR brings the package up to date with skeleton v0.2.0

Some notes/todos, probably for different PRs:

  • the design with LinearAlgebra.qr(::AbstractArray, ::Tuple, ::Tuple, ::Tuple) is both piracy and a large (40+) source of ambiguities, because LinearAlgebra tends to specialize according to qr(::StructuredArray, args...). Because I'm not so familiar with what this signature is specifically used for, I was wondering if it would be possible to only define this in terms of LinearAlgebra.qr(::AbstractArray, ::BlockPermutation) and do the other dispatch manually.
  • it wasn't clear to me if the intention for the LinearAlgebra extensions is to really be an extension, or simply defined in the main package.

@codecov
Copy link

codecov bot commented Dec 12, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@mtfishman
Copy link
Member

mtfishman commented Dec 12, 2024

Thanks for the comments, those are both good points to discuss. Can you raise those as separate issues?

As a short answer, that signature of qr is meant to be a labelled tensor version of QR, i.e. qr(a, (:i, :j, :k), (:i, :j), (:k)), but probably that can just make use of NamedDimsArrays, i.e. qr(nameddims(a, (:i, :j, :k)), (:i, :j), (:k,)), and the reason the signature is so generic is because we want to allow any objects as dimension names.

Another alternative is to use a wrapper type like Name to disambiguate in these cases, i.e. we could require inputs like qr(a, Name.((:i, :j, :k)), Name.((:i, :j)), Name.((:k,))), if you want to use dimension names/labels as opposed to BlockPermutation, which is a bit of a lower level interface.

@mtfishman mtfishman merged commit 74c6607 into ITensor:main Dec 12, 2024
11 checks passed
@lkdvos lkdvos deleted the skeleton branch December 12, 2024 13:46
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