Skip to content

[GradedAxes] Replace GradedAxes with GradedAxesNext #1355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 18, 2024
Merged

Conversation

mtfishman
Copy link
Member

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
Copy link
Member Author

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 mtfishman changed the title [WIP][GradedAxes] Replace GradedAxes with GradedAxesNext [GradedAxes] Replace GradedAxes with GradedAxesNext Mar 17, 2024
@mtfishman mtfishman marked this pull request as ready for review March 17, 2024 22:02
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

❗ Current head 0d95ddb differs from pull request most recent head 790f853. Consider uploading reports for the commit 790f853 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    #1355       +/-   ##
===========================================
- Coverage   84.40%   53.78%   -30.62%     
===========================================
  Files         100       99        -1     
  Lines        8581     8528       -53     
===========================================
- Hits         7243     4587     -2656     
- Misses       1338     3941     +2603     

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

@ogauthe
Copy link
Contributor

ogauthe commented Mar 18, 2024

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
Copy link
Contributor

ogauthe commented Mar 18, 2024

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)

@mtfishman
Copy link
Member Author

@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 mtfishman merged commit 70052f1 into main Mar 18, 2024
@mtfishman mtfishman deleted the GradedAxes branch March 18, 2024 12:35
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.

3 participants