Skip to content

Conversation

@TWiedemann
Copy link
Contributor

@TWiedemann TWiedemann commented Jan 21, 2026

  • Removes mp (moved points) from the input of the function because they can be recovered as MovedPoints(grp). Changes all function calls accordingly.
  • The function performs two random search loops: The first one looks for an n-cycle and a transposition, the second one conjugates the transposition from the first one by random group elements until some properties are satisfied. This PR separates these two loops, which avoids some unnecessary calls of PseudoRandom(grp).
  • Renames several variables so that their names are more telling.
  • Adds an documentation what exactly this function returns.
  • Adds comments that explain the algorithm.
  • Adds "TODO" comments in some related parts of the file where magic constants should be documented properly.
  • A few changes of the algorithm which, I think, enhance understandability. (E.g. I found the condition ForAll([1..QuoInt(n,2)-1],i->not IsBound(cyclen[2*i+1])) in the original version difficult to parse.)
  • Adds some tests.

I was unsure what the function should actually do in the cases n=1 and n=2. I decided that it returns fail, but it might also make sense to return an empty list for n=1 and the list containing the unique non-trivial element of grp if n=2. The version before the PR throws an error for SymmetricGroup(1) and returns [ (1,2), (1,2) ] for SymmetricGroup(2).

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.35%. Comparing base (9792788) to head (c132fe1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #369      +/-   ##
==========================================
- Coverage   71.55%   71.35%   -0.20%     
==========================================
  Files          43       43              
  Lines       18398    18409      +11     
==========================================
- Hits        13164    13136      -28     
- Misses       5234     5273      +39     
Files with missing lines Coverage Δ
gap/perm/giant.gi 55.57% <100.00%> (+1.54%) ⬆️

... and 3 files with indirect coverage changes

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

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.

1 participant