Skip to content

better IsAlmostSimple support in AllPrimitiveGroups - #79

Open
ThomasBreuer wants to merge 2 commits into
gap-packages:masterfrom
ThomasBreuer:TB_IsAlmostSimple
Open

better IsAlmostSimple support in AllPrimitiveGroups#79
ThomasBreuer wants to merge 2 commits into
gap-packages:masterfrom
ThomasBreuer:TB_IsAlmostSimple

Conversation

@ThomasBreuer

Copy link
Copy Markdown

Up to now, the groups had to be constructed and were checked. Now we use that IsAlmostSimple is determined by ONanScottType.

The IsAlmostSimpleGroup flag is now set in the groups returned by PrimitiveGroup.

(The primgrp package does not say which functions have special support in AllPrimitiveGroups, but is_almost_simple is listed in the Oscar documentation.)

Up to now, the groups had to be constructed and were checked.
Now we use that `IsAlmostSimple` is determined by `ONanScottType`.

The `IsAlmostSimpleGroup` flag is now set in the groups returned by
`PrimitiveGroup`.

(The primgrp package does not say which functions have special support,
but `is_almost_simple` is listed in the Oscar documentation.)
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.13%. Comparing base (dbae2c0) to head (3bad94f).

Files with missing lines Patch % Lines
lib/primitiv.gi 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   99.11%   99.13%   +0.01%     
==========================================
  Files          46       46              
  Lines       38324    38338      +14     
==========================================
+ Hits        37984    38005      +21     
+ Misses        340      333       -7     
Files with missing lines Coverage Δ
lib/primitiv.gi 60.34% <92.85%> (+2.19%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

(These are only tests for my additions,
most features of `AllPrimitiveGroups` do not have tests.)
@fingolfin

Copy link
Copy Markdown
Member

Thanks @ThomasBreuer! I let Claude have a closer look, and it pointed out three issues:

  1. Documentation: lib/primitiv.gd:166-172 does list which properties come from the database. IsAlmostSimpleGroup should be added there.
  2. CHANGES.md should be updated with this
  3. "values" can also be lists, which this does not support
AllPrimitiveGroups(NrMovedPoints,12,IsAlmostSimple,[true])       ->  0  (want 6)
AllPrimitiveGroups(NrMovedPoints,12,IsAlmostSimple,[true,false]) ->  0  (want 6)
AllPrimitiveGroups(NrMovedPoints,12,IsSimple,[true,false])       ->  6  (correct)

Wrong answer, no error. STGSelFunc already dispatches on true/false/
function/list, so delegating to it fixes all cases and removes both the
four-way branch and the hand-maintained type blacklist:

elif a=IsAlmostSimpleGroup or a=IsAlmostSimple then
  # for primitive groups, almost simple means O'Nan-Scott type 2
  bfunc:= b;
  b:= val -> STGSelFunc(val = "2", bfunc);
  gut[i]:=Filtered(gut[i],j->STGSelFunc(PRIMGrp(i,j)[4],b));

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.

2 participants