Skip to content

Conversation

@lkdvos
Copy link
Member

@lkdvos lkdvos commented Feb 12, 2025

I added the utility rank and cond functions, since I thought this would help fix #219 . I overlooked that there S is already a vector, so that actually didn't help, but it might be useful anyways?

To do

  • Add tests

@codecov
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.51%. Comparing base (511fa90) to head (1c2dd61).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/tensors/linalg.jl 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #220      +/-   ##
==========================================
+ Coverage   82.44%   82.51%   +0.06%     
==========================================
  Files          43       43              
  Lines        5536     5552      +16     
==========================================
+ Hits         4564     4581      +17     
+ Misses        972      971       -1     

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

@Jutho
Copy link
Member

Jutho commented Feb 13, 2025

Looks good to me. Is the "add tests" an actual todo?

@lkdvos
Copy link
Member Author

lkdvos commented Feb 13, 2025

What is an "actual" to do? I would definitely say "should do"...

@Jutho
Copy link
Member

Jutho commented Feb 17, 2025

I am making some changes (the condition number of the map on an empty space should be one, not zero. Condition numbers lie in the range [1, +inf]). But now for the difficult question: should rank count the degeneracy, i.e. is it sum(dim(c) * rank(b)) or simply sum(rank(b)) ?

@lkdvos
Copy link
Member Author

lkdvos commented Feb 17, 2025

With regards to the condition number, I took this from LinearAlgebra, see also this discussion and the related MATLAB discussion. I have no opinion about this I think, whatever you feel is most consistent is good to me.

For the rank, I guess if the matrix is full rank, you would expect rank(A) = dim(domain(A)), so whatever is consistent with that?

@Jutho
Copy link
Member

Jutho commented Feb 17, 2025

Ok I see regarding condition number. Not my favorite pick, but I can live with it for consistency.

For the rank, that choice would include quantum dimensions, which I am definitely happy with as we also do this in other places.

Committer: Jutho Haegeman <[email protected]>
@Jutho
Copy link
Member

Jutho commented Feb 17, 2025

@lkdvos , if the tests pass, this is good to go for me.

@lkdvos
Copy link
Member Author

lkdvos commented Feb 17, 2025

Nightly test failures are unrelated, so I'm merging this now.

@lkdvos lkdvos merged commit a4eb3f3 into master Feb 17, 2025
12 of 14 checks passed
@lkdvos lkdvos deleted the rank branch August 26, 2025 13:30
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.

MethodError in svd_pullback!

3 participants