Improve performance of bracket for AbstractLieAlgebraElem
#4228
Conversation
lgoettgens
added
topic: LieAlgebras
experimental
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4228 +/- ##
=======================================
Coverage 84.58% 84.58%
=======================================
Files 631 631
Lines 85009 85016 +7
=======================================
+ Hits 71908 71915 +7
Misses 13101 13101
|
| iszero(cxi) && continue | ||
| for (j, cyj) in enumerate(coefficients(y)) | ||
| iszero(cyj) && continue | ||
| Hecke.add_scaled_row!(_struct_consts(L)[i, j], vec, cxi * cyj) |
There was a problem hiding this comment.
I double checked and it seems this is indeed the correct argument order -- very confusing when one already knows addmul! :-(.
On the up side, perhaps we can just simply define methods
addmul!(a::SRow{T}, b::SRow{T}, c::T) where T = add_right_scaled_row(b, a, c)
addmul!(a::SRow{T}, b::T, c::SRow{T}) where T = add_left_scaled_row(c, a, b)and migrate to those over time?
But of course that's not any reason to hold up this PR, just musings :-)
There was a problem hiding this comment.
See thofma/Hecke.jl#1659. I would suggest to merge this PR here right now and then once thofma/Hecke.jl#1659 is available go through Oscar and phase out the add_scaled_row! calls. (if you agree please approve and merge this)
by being a bit smarter with handling sparse rows