Skip to content

LieAlgebras: Introduce WeightLattice#4344

Merged
joschmitt merged 9 commits intomasterfrom
lg/weight-lattice
Nov 27, 2024
Merged

LieAlgebras: Introduce WeightLattice#4344
joschmitt merged 9 commits intomasterfrom
lg/weight-lattice

Conversation

@lgoettgens
Copy link
Copy Markdown
Member

@lgoettgens lgoettgens commented Nov 25, 2024

Changes:

  • Introduce WeightLattice, a formal parent for WeightLatticeElem
  • character and friends now return a Dict{WeightLatticeElem, T} (with default T = Int)
  • Weyl actions: remove some duplicate ugliness; instead add a warning to the docs page

preview: https://docs.oscar-system.org/previews/PR4344/Experimental/LieAlgebras/

@lgoettgens lgoettgens added topic: lie theory experimental Only changes experimental parts of the code labels Nov 25, 2024
@lgoettgens lgoettgens force-pushed the lg/weight-lattice branch 2 times, most recently from d55c9cd to 5d48fee Compare November 25, 2024 15:03
@lgoettgens lgoettgens marked this pull request as ready for review November 25, 2024 15:36
@benlorenz benlorenz closed this Nov 26, 2024
@benlorenz benlorenz reopened this Nov 26, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 85.76512% with 40 lines in your changes missing coverage. Please review.

Project coverage is 84.31%. Comparing base (9da6d04) to head (faf6df4).
Report is 221 commits behind head on master.

Files with missing lines Patch % Lines
experimental/LieAlgebras/src/WeightLattice.jl 87.56% 23 Missing ⚠️
experimental/LieAlgebras/src/LieAlgebraModule.jl 51.72% 14 Missing ⚠️
experimental/LieAlgebras/src/RootSystem.jl 92.59% 2 Missing ⚠️
...xperimental/LieAlgebras/test/WeightLattice-test.jl 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4344      +/-   ##
==========================================
- Coverage   84.32%   84.31%   -0.02%     
==========================================
  Files         649      651       +2     
  Lines       86430    86483      +53     
==========================================
+ Hits        72880    72915      +35     
- Misses      13550    13568      +18     
Files with missing lines Coverage Δ
...BasisLieHighestWeight/src/BasisLieHighestWeight.jl 100.00% <ø> (ø)
...imental/BasisLieHighestWeight/src/MainAlgorithm.jl 97.76% <100.00%> (ø)
...l/BasisLieHighestWeight/test/MainAlgorithm-test.jl 97.50% <ø> (ø)
experimental/LieAlgebras/src/LieAlgebras.jl 100.00% <ø> (ø)
experimental/LieAlgebras/src/Types.jl 96.73% <100.00%> (+0.05%) ⬆️
experimental/LieAlgebras/src/WeylGroup.jl 90.25% <100.00%> (-0.03%) ⬇️
...rimental/LieAlgebras/test/LieAlgebraModule-test.jl 100.00% <100.00%> (ø)
experimental/LieAlgebras/test/RootSystem-test.jl 97.16% <100.00%> (ø)
experimental/LieAlgebras/test/WeylGroup-test.jl 97.77% <ø> (ø)
...xperimental/LieAlgebras/test/WeightLattice-test.jl 92.30% <92.30%> (ø)
... and 3 more

@lgoettgens
Copy link
Copy Markdown
Member Author

lgoettgens commented Nov 26, 2024

can we get this merged? @fingolfin or someone else?

@lgoettgens
Copy link
Copy Markdown
Member Author

Re-drafting until conflicts with #4339 are resolved.

@lgoettgens lgoettgens marked this pull request as draft November 26, 2024 17:56
@lgoettgens lgoettgens force-pushed the lg/weight-lattice branch 2 times, most recently from 5d19aab to faf6df4 Compare November 27, 2024 11:44
@lgoettgens
Copy link
Copy Markdown
Member Author

lgoettgens commented Nov 27, 2024

CI is happy with my merge resolution. So this is again ready for merge

@lgoettgens lgoettgens marked this pull request as ready for review November 27, 2024 12:48
Comment on lines +10 to +14
!!! warning
Weyl groups in OSCAR afford both left and right actions on roots and weights.
Note however, that **the left action** is the default (to align with the literature),
and all more complex functionality is defined with respect to the left action, e.g.
[`conjugate_dominant_weight_with_elem(::WeightLatticeElem)`](@ref).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is none of my business, but I spent too much time transposing-matrices-until-the-result-looks-correct to not comment here:
So far, my understanding was that all group actions are from the right in OSCAR (at least by default) and we happily break textbook conventions at least in the linear algebra and invariant theory to have this consistent. Why not here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short answer: this is still in discussion with Max (before this migrates to src/). This is the current state of things, and I wanted to document how it is right now (but subject to change). The removal of some *_left_* and *_right_* functions in this PR is to clean things up that partially made it possible to do both action directions, but this didn't seem like a healthy way forward (and it didn't think of all places where this was an issue). So I would like to clean this up again, further discuss with Max and afterwards tackle this more systematically.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks!

Copy link
Copy Markdown
Member

@joschmitt joschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Aachen people seem to be happy with it.

@joschmitt joschmitt merged commit 3af3c91 into master Nov 27, 2024
@joschmitt joschmitt deleted the lg/weight-lattice branch November 27, 2024 13:12
@lgoettgens lgoettgens added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experimental Only changes experimental parts of the code release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: lie theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants