Streamline powers of Lie algebra modules #3081

lgoettgens · 2023-12-07T16:35:49Z

Make the code very similar to the exterior powers of ModuleFP by @HechtiDerLachs.

exterior powers
symmetric powers
tensor powers
cleanup of generic "power code"

Furthermore, fix some type instabilities in the exterior power code of ModuleFP.

HechtiDerLachs

Looks good. As already said: At some point we need to streamline the multiplication map.

experimental/LieAlgebras/src/LieAlgebraModule.jl

src/Modules/ExteriorPowers/Generic.jl

lgoettgens · 2023-12-21T16:44:52Z

This PR is now ready to be reviewed and merged from my POV. There is still some stuff do do around direct sums and tensor products, but I would like to postpone that to a future PR that tackles #3059 for Lie algebra modules.

@HechtiDerLachs could you please in particular have a look at the changes in src/? It should only be the addition of type assertions where they are needed for inference to be more successful. You can see the result when calling @code_warntype exterior_power(F, 2) where F is a free module and comparing that to the latest master.

codecov · 2023-12-22T11:53:15Z

Codecov Report

Merging #3081 (8afcbeb) into master (c26a436) will increase coverage by 0.12%.
Report is 7 commits behind head on master.
The diff coverage is 90.84%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3081      +/-   ##
==========================================
+ Coverage   81.14%   81.27%   +0.12%     
==========================================
  Files         556      556              
  Lines       73894    74005     +111     
==========================================
+ Hits        59963    60144     +181     
+ Misses      13931    13861      -70

Files	Coverage Δ
...eAndHyperComplexes/src/Morphisms/linear_strands.jl	`95.23% <100.00%> (ø)`
...perComplexes/src/Morphisms/simplified_complexes.jl	`95.49% <100.00%> (ø)`
experimental/LieAlgebras/src/LieAlgebras.jl	`100.00% <ø> (ø)`
...rimental/LieAlgebras/test/LieAlgebraModule-test.jl	`100.00% <100.00%> (ø)`
src/Modules/ExteriorPowers/Generic.jl	`93.22% <100.00%> (ø)`
src/Modules/ModulesGraded.jl	`81.78% <100.00%> (ø)`
src/Modules/UngradedModules/FreeModElem.jl	`96.84% <100.00%> (ø)`
src/Modules/UngradedModules/SubquoModule.jl	`74.37% <100.00%> (ø)`
src/Modules/UngradedModules/SubquoModuleElem.jl	`84.92% <100.00%> (ø)`
src/Modules/ExteriorPowers/FreeModules.jl	`98.38% <86.66%> (-1.62%)`	⬇️
... and 4 more

... and 13 files with indirect coverage changes

d

`exterior_power(::FreeMod)` now completely stable, for `SubquoModule` only partially

direct sum and tensor product still missing

many new changes

lgoettgens · 2024-01-08T10:01:36Z

bump @HechtiDerLachs

src/Modules/ExteriorPowers/FreeModules.jl

HechtiDerLachs · 2024-01-12T15:24:07Z

src/Modules/ExteriorPowers/Generic.jl

+@attr Dict{Int, Tuple{typeof(F), MapFromFunc}} function _exterior_powers(F::ModuleFP)
+  return Dict{Int, Tuple{typeof(F), MapFromFunc}}()


Suggested change

@attr Dict{Int, Tuple{typeof(F), MapFromFunc}} function _exterior_powers(F::ModuleFP)

return Dict{Int, Tuple{typeof(F), MapFromFunc}}()

@attr Dict{Int, Tuple{typeof(F), <:MapFromFunc}} function _exterior_powers(F::ModuleFP)

return Dict{Int, Tuple{typeof(F), MapFromFunc}}()

This might be a bit too restrictive. Maybe, one day we change the return type. And they are both abstract, so no harm is done staying with just Map right?

Tuples are invariant in julia, thus the <: shouldn't be there.
As all of this is purely internal, we can change that to Map back anytime we want. But making it as concrete as possible helps type inference massively. In this particular case, it can use some of the annotations in the MapFromFunc code in Hecke

src/Modules/ExteriorPowers/SubQuo.jl

Co-authored-by: Matthias Zach <[email protected]>

HechtiDerLachs · 2024-01-16T15:14:14Z

Tests still fail. Other than that, I can approve for my part. Let me know when you're ready.

lgoettgens · 2024-01-16T17:33:14Z

src/Modules/ModulesGraded.jl

@@ -513,7 +513,7 @@ function degree(f::FreeModElem)
  iszero(f) && return A[0]
  f.d = isa(f.d, GrpAbFinGenElem) ? f.d : determine_degree_from_SR(coordinates(f), degrees(parent(f)))
  isa(f.d, GrpAbFinGenElem) || error("The specified element is not homogeneous.")
-  return f.d
+  return f.d::GrpAbFinGenElem


btw this was the culprit

lgoettgens · 2024-01-16T17:34:26Z

I found a small fix for the failure, that made it possible to revert some other changes to enhance readability.
Let's hope that I broke nothing else.

lgoettgens · 2024-01-17T08:09:29Z

Tests still fail. Other than that, I can approve for my part. Let me know when you're ready.

CI now looks good

HechtiDerLachs

I think the style adjustment is not worth running the tests again. So if there's nothing else, go ahead with the merge from my side.

HechtiDerLachs · 2024-01-17T10:51:58Z

src/Modules/ExteriorPowers/SubQuo.jl

-  R = base_ring(M)
-  ((p >= 0) && (p <= n)) || error("exponent out of range")
+  R = base_ring(M)::base_ring_type(M)
+  @req 0 <= p <= n "Exponent out of bounds"


Suggested change

@req 0 <= p <= n "Exponent out of bounds"

@req 0 <= p <= n "exponent out of bounds"

lgoettgens added WIP NOT ready for merging topic: LieAlgebras labels Dec 7, 2023

lgoettgens marked this pull request as draft December 7, 2023 16:36

HechtiDerLachs previously approved these changes Dec 8, 2023

View reviewed changes

experimental/LieAlgebras/src/LieAlgebraModule.jl Outdated Show resolved Hide resolved

experimental/LieAlgebras/src/LieAlgebraModule.jl Outdated Show resolved Hide resolved

src/Modules/ExteriorPowers/Generic.jl Outdated Show resolved Hide resolved

lgoettgens force-pushed the lg/lie-powers branch 8 times, most recently from da83aab to 6099be7 Compare December 21, 2023 16:14

lgoettgens removed the WIP NOT ready for merging label Dec 21, 2023

lgoettgens requested a review from HechtiDerLachs December 21, 2023 16:44

lgoettgens marked this pull request as ready for review December 21, 2023 18:06

lgoettgens added 13 commits January 4, 2024 13:19

Make exterior_power(::ModuleFP) slightly more type stable

45c2036

Make exterior powers of Lie modules compatible with ModuleeFP

e683bb6

d

Remove splatting in my_mult

3d01875

More type stability for exterior powers

f721e54

`exterior_power(::FreeMod)` now completely stable, for `SubquoModule` only partially

Rework induced_map_on_exterior_power

a044259

Change symmetric_power analogous to the exterior_power change

ef46cc1

Change tensor_power analogous to the exterior_power change

ddf0eb5

Cleanup hom_power

59ded76

Minor changes to standard_module and dual

5384a67

Make is_direct_sum and is_tensor_product consistent and some cleanup

60e4069

Streamline some caching

f8c92ad

direct sum and tensor product still missing

Diversify module conformance tests

4375462

Streamline parent object calling

5bd052f

lgoettgens force-pushed the lg/lie-powers branch 2 times, most recently from d02b98f to a47ce6b Compare January 4, 2024 12:44

Fix tests

a47ce6b

lgoettgens closed this Jan 9, 2024

lgoettgens reopened this Jan 9, 2024

lgoettgens added 2 commits January 12, 2024 15:34

Merge branch 'master' into lg/lie-powers

f5d0b6f

Fix Aqua test

68545ce

HechtiDerLachs reviewed Jan 12, 2024

View reviewed changes

Apply suggestions from code review

eb66cc8

Co-authored-by: Matthias Zach <[email protected]>

Fix tests

8afcbeb

lgoettgens commented Jan 16, 2024

View reviewed changes

HechtiDerLachs approved these changes Jan 17, 2024

View reviewed changes

lgoettgens added the merge me label Jan 17, 2024

fingolfin approved these changes Jan 17, 2024

View reviewed changes

fingolfin merged commit bc93413 into oscar-system:master Jan 17, 2024
25 checks passed

lgoettgens deleted the lg/lie-powers branch January 17, 2024 14:53

lgoettgens removed the merge me label Jan 17, 2024

thofma mentioned this pull request Feb 4, 2024

Bump Hecke version, fix some regressions #3311

Merged

thofma mentioned this pull request Feb 20, 2024

feat: add regular_character #3393

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Streamline powers of Lie algebra modules #3081

Streamline powers of Lie algebra modules #3081

lgoettgens commented Dec 7, 2023 •

edited

Loading

HechtiDerLachs left a comment

lgoettgens commented Dec 21, 2023

codecov bot commented Dec 22, 2023 •

edited

Loading

lgoettgens commented Jan 8, 2024

HechtiDerLachs Jan 12, 2024

lgoettgens Jan 12, 2024

HechtiDerLachs commented Jan 16, 2024

lgoettgens Jan 16, 2024

lgoettgens commented Jan 16, 2024

lgoettgens commented Jan 17, 2024

HechtiDerLachs left a comment

HechtiDerLachs Jan 17, 2024

		@attr Dict{Int, Tuple{typeof(F), MapFromFunc}} function _exterior_powers(F::ModuleFP)
		return Dict{Int, Tuple{typeof(F), MapFromFunc}}()

	@req 0 <= p <= n "Exponent out of bounds"
	@req 0 <= p <= n "exponent out of bounds"

Streamline powers of Lie algebra modules #3081

Streamline powers of Lie algebra modules #3081

Conversation

lgoettgens commented Dec 7, 2023 • edited Loading

HechtiDerLachs left a comment

Choose a reason for hiding this comment

lgoettgens commented Dec 21, 2023

codecov bot commented Dec 22, 2023 • edited Loading

Codecov Report

lgoettgens commented Jan 8, 2024

HechtiDerLachs Jan 12, 2024

Choose a reason for hiding this comment

lgoettgens Jan 12, 2024

Choose a reason for hiding this comment

HechtiDerLachs commented Jan 16, 2024

lgoettgens Jan 16, 2024

Choose a reason for hiding this comment

lgoettgens commented Jan 16, 2024

lgoettgens commented Jan 17, 2024

HechtiDerLachs left a comment

Choose a reason for hiding this comment

HechtiDerLachs Jan 17, 2024

Choose a reason for hiding this comment

lgoettgens commented Dec 7, 2023 •

edited

Loading

codecov bot commented Dec 22, 2023 •

edited

Loading