[ITensors] Customisable Arrow directions in SVD

[JoeyT1994]

Description

This PR adds customisable arrow directions to the internal bonds on the S tensor in the SVD. Moreover the factorize_svd() function has been changed when ortho = "None" so that it works correctly with QNs and takes a proper square root of S. Tests are included for both fermionic and spin QNS. With the fermionic QNS working due to the ability to customise the arrows and set the arrows on S to ITensors.Out (currently this does not appear to work if the arrow directions are mixed).

How Has This Been Tested?

Additional tests added to test/base/test_svd.jl which check that svd() and factorize_svd() give correct decompositions of an ITensor when custom arrow directions are used

[JoeyT1994]

Okay I changed the tests to work for randomITensors instead of using those made from an MPS. Also moved the fermion test to test_fermions.jl

[emstoudenmire]

Looks like good code, but I have some questions about the philosophy here of allowing the arrows to be set from outside. Joey (and Matt if you want), could we discuss sometime soon to see if this is the best solution for fixing the fermionic case?

My main concern is that allowing custom arrows on S could result in U (or V) having arrows that don't go "through" but instead both go in or both go out (both in the case of SVD'ing a matrix so that U and V have two indices, I mean). That could be ok if the quantum numbers have appropriately flipped signs to keep the flux of U and V to be zero, but it seems much more natural to me to have the arrows just go through so that the QN's are the same across both the in and out indices.

[mtfishman]

I agree that hopefully there is a more general/fundamental fix for the fermionic case so that this isn't strictly necessary for that, but I don't see why this shouldn't be allowed more broadly, i.e. I could definitely imagine cases where it would be nice to control the arrow direction.

[mtfishman]

I think you can always choose the QNs appropriately so that U and V have zero flux, no matter what arrow directions are chosen, and we will still keep that convention (@JoeyT1994 have you checked that?).

[mtfishman]

An example which we came across is if you are doing a factorization like:

A, B = U * S, V
A, B = U, S * V
A, B = U * sqrt(S), sqrt(S) * V

it's nice to be able to choose whether the arrow ends up pointing from A to B or vice versa.

[emstoudenmire]

I'm fine with it if we mainly view it as an advanced code capability, to be used in rare cases where the user knows what they are doing. It's just that I would hope we could find solutions where you'd never have to resort to using it, because we could provide good defaults (with an exception possibly being when there are mixed arrows on U or V, in which case any kind of automatic arrow determination becomes ambiguous).

Sticking to the matrix cases where the original tensor T has two indices, one going onto U and the other onto V, the nice thing about having S mimic the original arrow directions of T is that, in addition to U and V having "through" arrows, which is the most natural case for zero-flux tensors and unitary or isometric maps, if one forms (US) or (SV), each of these will have the same arrow structure as T also.

[mtfishman]

I agree that the current defaults are good and most of the time customizing the arrow directions should not be used.

[JoeyT1994]

Yes happy to discuss! The default on svd() is for the directions to be set to nothing and so it will just default the combiner arrows to what they were? And having the arrows `go through' makes the most sense and there should be no need to customise the arrow dirn.

Nonetheless I do think it is nice to have it customisable as an advanced usage case.

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d4df519) 85.39% compared to head (785ceb1) 66.63%.

❗ Current head 785ceb1 differs from pull request most recent head b242aef. Consider uploading reports for the commit b242aef to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1197       +/-   ##
===========================================
- Coverage   85.39%   66.63%   -18.77%     
===========================================
  Files          89       87        -2     
  Lines        8430     7868      -562     
===========================================
- Hits         7199     5243     -1956     
- Misses       1231     2625     +1394     

Files	Coverage Δ
src/tensor_operations/matrix_decomposition.jl	91.75% <100.00%> (-0.59%)	⬇️
src/tensor_operations/matrix_algebra.jl	92.00% <75.00%> (-3.35%)	⬇️

... and 64 files with indirect coverage changes

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

[JoeyT1994]

@mtfishman I just changed this to restrict to just specifying one arrow direction when calling factorize_svd(). I'm not sure what to call the kwarg though? Dir is already taken as a deprecated keyword argument.

[mtfishman]

I think we can repurpose dir for this use case, it has been erroring for a while now so people shouldn't be using that.

[JoeyT1994]

Okay I repurposed it and commented out the deprecated warning

[mtfishman]

Some tests are failing which check for bad inputs for specifying the svd_alg backend, which should be getting forwarded as the alg kwarg to svd through factorize_svd. Maybe the current code logic is not forwarding that keyword argument properly.

[JoeyT1994]

Ah I think I found the bug and should be fixed now!

[mtfishman]

Looks good, thanks!