Skip to content
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

Add some more functionality, docs and tests for root systems #3191

Merged
merged 11 commits into from
Jan 23, 2024

Conversation

lgoettgens
Copy link
Member

No description provided.

@felix-roehrich
Copy link
Collaborator

Please add a property test to make sure that the roots of the dual root system correspond to the coroots of the base root system.

Returns the `i`-th coroot of `R`, i.e. the `i`-th root of the dual root system of `R`.
This is a more efficient version for `coroots(R)[i]`.

Also see: `coroots`.
Copy link
Member

Choose a reason for hiding this comment

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

How about using an actual ref?

Suggested change
Also see: `coroots`.
Also see: [`coroots`](@ref).

Copy link
Member Author

Choose a reason for hiding this comment

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

This does only work once we have a markdown page collecting the docstrings. That's something @felix-roehrich wants to approach in the near future.

@lgoettgens
Copy link
Member Author

Please add a property test to make sure that the roots of the dual root system correspond to the coroots of the base root system.

done

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

Merging #3191 (f0e36cd) into master (a7f9f88) will increase coverage by 0.00%.
The diff coverage is 56.66%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #3191    +/-   ##
========================================
  Coverage   81.27%   81.27%            
========================================
  Files         556      557     +1     
  Lines       74019    74133   +114     
========================================
+ Hits        60158    60253    +95     
- Misses      13861    13880    +19     
Files Coverage Δ
experimental/LieAlgebras/src/CartanMatrix.jl 98.16% <100.00%> (-0.01%) ⬇️
experimental/LieAlgebras/src/LieAlgebras.jl 100.00% <ø> (ø)
experimental/LieAlgebras/test/CartanMatrix-test.jl 95.07% <100.00%> (+0.10%) ⬆️
experimental/LieAlgebras/test/RootSystem-test.jl 100.00% <100.00%> (ø)
experimental/LieAlgebras/src/RootSystem.jl 51.25% <45.83%> (+10.22%) ⬆️

... and 1 file with indirect coverage changes

lgoettgens and others added 2 commits January 18, 2024 20:58
Co-authored-by: Felix Röhrich <47457568+felix-roehrich@users.noreply.github.com>
@lgoettgens
Copy link
Member Author

can we get a merge of this? maybe @thofma ?

@thofma thofma enabled auto-merge (squash) January 23, 2024 10:34
@thofma thofma merged commit e5a2cdf into oscar-system:master Jan 23, 2024
24 of 25 checks passed
@lgoettgens lgoettgens deleted the lg/root-system-stuff branch January 23, 2024 10:35
ooinaruhugh pushed a commit to ooinaruhugh/Oscar.jl that referenced this pull request Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: LieAlgebras
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants