ITensors.jl icon indicating copy to clipboard operation
ITensors.jl copied to clipboard

[WIP][GradedAxes] Replace `GradedAxes` with `GradedAxesNext`

Open mtfishman opened this issue 1 year ago • 1 comments

This is a followup to #1351.

I still need to implement a few missing functions for the new GradedUnitRange type related to fusion and sorting by sector/block labels.

@ogauthe

mtfishman avatar Mar 15 '24 17:03 mtfishman

This is pretty close to being ready, in the latest I've implemented taking the naive tensor product of two graded axes (for abelian symmetries, which was all that was implemented in the previous version of GradedAxes). I need to fix a final issue with calling fusedims on a BlockSparseArray that has graded axes with abelian sectors.

mtfishman avatar Mar 15 '24 21:03 mtfishman

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.78%. Comparing base (12fbcc2) to head (0d95ddb).

:exclamation: Current head 0d95ddb differs from pull request most recent head 790f853. Consider uploading reports for the commit 790f853 to get more accurate results

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

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1355       +/-   ##
===========================================
- Coverage   84.40%   53.78%   -30.62%     
===========================================
  Files         100       99        -1     
  Lines        8581     8528       -53     
===========================================
- Hits         7243     4587     -2656     
- Misses       1338     3941     +2603     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Mar 17 '24 22:03 codecov-commenter

As I understand, you redefined GradedUnitRange as a BlockedUnitRange with an extra vector of LabelledInteger to label each sector. I understand this design is simpler, I was previously wondering why a GradedUnitRange was not a subtype of BlockedUnitRange (I suppose the answer is BlockedUnitRange was too concrete).

In doing so you removed the concept of isdual. I suppose that for a BlockedUnitRange, this concept does not make particularly sense and probably adds complexity. However from a symmetry point of view, this removes a crucial information from the GradedUnitRange. If it is not in a GradedUnitRange, it has to be somewhere else.

I see 4 possibilities for isdual:

  1. putting it inside the label. It would be possible to create a GradedUnitRange with different isdual values for different sectors. I really don't like this option: from a coding perspective, allowing for an axis with seemingly different sectors U1(1), isdual=False and U1(-1), isdual=True appears to me as extremely risky. From a mathematical perspective, it makes little sense to mix dual and non-dual sectors in what is supposed to be a single vector space.

  2. defining a new object "Representation" or "Space", that would encode the information of a reducible representation, with sectors, multiplicities and a single isdual::Bool. It would not convey the concept of a range, but would impose unique, sorted labels with a well defined arrow direction. We previously ruled out this option as it would be a doublet for GradedUnitRange in many aspects.

  3. adding a field dual_axis::NTuple{N,Bool} as a field for FusionTensor, meaning the arrow direction is stored independently from the axis itself. It works, but is not the most convenient, as it require to specify this argument at many different places.

  4. coming back to the previous implementation, as a field of a GradedUnitRange.

Overall, I believe that without the isdual information, GradedUnitRange cannot act as a complete implementation of the concept of "Vector space". Options 1), 2) and 3) boil down to splitting the two concepts: 1) just means that a Vector space can only contain a single sector and a GradedUnitRange is a collection of spaces. Option 3) means a vector space is stored as 2 independent components.

ogauthe avatar Mar 18 '24 01:03 ogauthe

It seems to me that no solution is optimal and a tradeoff needs to be made. To specify my position, I currently lean towards option 2), explicitly splitting the two concepts and implementing both. Indeed the two concepts are related, but they do not correspond exactly to the same use case:

  • a "Range" is relevant to index an array, with an intuitive notion of length and blocklength. It imposes no constraint on the labels, they can be unsorted or repeated. Fusing two ranges corresponds to taking the Cartesian product of them, and no permutation is needed. It should handle indexing BlockSparseMatrix but also abelian tensors in the BlockSparseTensor format.

  • a "Space" is associated with the concept of symmetry. It carries sectors and degeneracies, with sectors being unique (and ideally sorted). It has a dimension, which is not related to the length of the range in a simple way, and not even an integer in some cases. Fusing two spaces is ruled by the fusion ring.

It is easy to construct a Range from a Space, however the reverse may not be possible or imposes to set implicit conventions.

EDIT: independently from this PR, last week I reached the conclusion that implementing Sectors and "Space" separately was awkward, as the two are intrinsically linked. In most cases, a Sector should behave exactly as a "Space", basically the only moment where it cannot is to define fusion rules and to compute a dimension (one could even get rid of dual(Sector), the only thing really needed is the sorting order it defines)

ogauthe avatar Mar 18 '24 01:03 ogauthe

@ogauthe let's discuss this issue around dual offline. I was planning to address that issue in a future PR, and merge this PR as-is as a starting point for future work, including deciding how to re-incorporate the concept of contravariance/covariance. I agree there are a few different options for that, with various costs and benefits.

mtfishman avatar Mar 18 '24 12:03 mtfishman