-
Notifications
You must be signed in to change notification settings - Fork 120
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
LieAlgebras: Enhance module interface #2262
LieAlgebras: Enhance module interface #2262
Conversation
e66fc89
to
a523ab3
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2262 +/- ##
==========================================
+ Coverage 73.47% 73.57% +0.09%
==========================================
Files 367 367
Lines 50277 50384 +107
==========================================
+ Hits 36940 37068 +128
+ Misses 13337 13316 -21
|
d15ac2d
to
ae935c9
Compare
1a874fd
to
d05cc4e
Compare
|
||
function base_modules(V::LieAlgebraModule{C}) where {C<:RingElement} | ||
@req is_direct_sum(V) "Not a direct sum module." | ||
return get_attribute(V, :base_modules)::Vector{LieAlgebraModule{C}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I changed it from an abstract Tuple{Vararg{LieAlgebraModule{C}}}
to a concrete Vector{LieAlgebraModule{C}}
. @fingolfin
d05cc4e
to
ad1e1f8
Compare
ad1e1f8
to
2f31181
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me
Currently, there is some weird behavior when two identical modules have been constructed in different ways, e.g. some module and its 1st exterior power.
This PR first adds tests modeling the intended behavior and then approaches to fix the above accordingly.
While on my way, I may add some more module constructions.