Skip to content

Fix SL godown eigenspace dimension check#516

Merged
fingolfin merged 2 commits into
masterfrom
codex/fix-sln-godown-eigenspace
Jul 14, 2026
Merged

Fix SL godown eigenspace dimension check#516
fingolfin merged 2 commits into
masterfrom
codex/fix-sln-godown-eigenspace

Conversation

@fingolfin

Copy link
Copy Markdown
Member

This prevents non-invariant spaces from reaching LinearAction.

Co-authored-by: Codex codex@openai.com

This prevents non-invariant spaces from reaching LinearAction.

Co-authored-by: Codex <codex@openai.com>
@fingolfin

fingolfin commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

This fixes a very rare error, but it occurred in PR #512 (due to bad luck).

Codex came up with a slightly different "fix" first, but after some back and forth, we settled on this. I think the main difference is that before, it could happen that all eigenspaces have dimension 1, but we really also want an eigenspace of dimension 3 (when d=4, which is the only case we actually use, I believe).

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.51%. Comparing base (f6b57ad) to head (607d771).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #516   +/-   ##
=======================================
  Coverage   86.51%   86.51%           
=======================================
  Files          45       45           
  Lines       19262    19262           
=======================================
  Hits        16665    16665           
  Misses       2597     2597           
Files with missing lines Coverage Δ
gap/projective/sl.gi 98.41% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread gap/projective/sl.gi Outdated
gap> seed:=1;;
gap> Reset(GlobalMersenneTwister, seed);;
gap> Reset(GlobalRandomSource, seed);;
gap> h := GL(6,8);;

@Till-Eisen Till-Eisen Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I could see the case $d-1 = dim-d$ as a problem in the previous code. In this case there might have been "to many" eigenspaces of dimension 1.
Perhaps $GL(7,8)$ instead of $GL(6,8)$ would be a good test as well, since then $dim=7, d= 4$ and thereby $dim-d=d-1$?

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.

In fact also $dim=5$ is of interest, too, as then $dim-d=1$

Co-authored-by: Torben Wiedemann <7226617+TWiedemann@users.noreply.github.com>
@fingolfin
fingolfin merged commit c7a7183 into master Jul 14, 2026
5 checks passed
@fingolfin
fingolfin deleted the codex/fix-sln-godown-eigenspace branch July 14, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants