Fix and enhancements to isomorphism test - #6544
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6544 +/- ##
==========================================
+ Coverage 78.98% 79.00% +0.02%
==========================================
Files 684 684
Lines 294205 294246 +41
Branches 8647 8670 +23
==========================================
+ Hits 232370 232480 +110
+ Misses 60028 59959 -69
Partials 1807 1807 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # nevertheless faster.. | ||
|
|
||
| #AsIsomorphism | ||
| InstallGlobalFunction(AsAutomorphism, |
There was a problem hiding this comment.
That doesn't match our naming conventions, it should be, say,
| InstallGlobalFunction(AsAutomorphism, | |
| InstallGlobalFunction(OnSubgroupsByAutomorphism, |
(The rds package already defines an OnSubgroups action, albeit a bit differently, so we can't just use that)
There was a problem hiding this comment.
I know that the name is nonstandard. But this action could be not just on subgroups, but on elements. Maybe OnGroupsAsAutomorphism?
|
With this pull request: |
Add a documented global function `AsAutomorphism(sub,autom)`, replacing the local `asAutom`/`asAutomorphism` helpers that were duplicated in `AutomorphismGroup` and `PatheticIsomorphism`. Factor the normal subgroup class stabilization into a local function `stabilizeSubgroupClass`, using permutation action and set stabilizer instead of `OrbitStabilizerAlgorithm`, lower the index threshold to 10^3 for using it. Keep the inner automorphism group as permutations for the condition search. When finding the isomorphism through multiple orbits and stabilizers, reduce the number of generators. Clean out some dead code and careless double computation. Ensure existing permrep is used for stabilizer. Avoid searching hard for 2 generators if the cost seems high.
Fixed. (But please note that |
Thanks a lot, now the code works. Unfortunately, I get the following timings in a related example.
The time increase seems to be due to a longer list of generators in the result of I hope that the longer generators list is not really needed. (In my examples, the actual results of |
Ensure the subgroup found so far remains a subgroup of the search space. This fixes the bug from #6537.
Add a documented global function
AsAutomorphism(sub,autom), replacing thelocal
asAutom/asAutomorphismhelpers that were duplicated inAutomorphismGroupandPatheticIsomorphism.Factor the normal subgroup class stabilization into a local function
stabilizeSubgroupClass, using permutation action and set stabilizerinstead of
OrbitStabilizerAlgorithm, lower the index threshold to 10^3 forusing it.
When finding the isomorphism through multiple orbits and stabilizers, reduce
the number of generators.
Clean out some dead code and careless double computation.
Avoid searching hard for special case of 2 generators with few images. if the cost seems high a priori — tune down the special case selection.
Release notes:
A bug in the isomorphism test for groups was fixed, overall performance of the test was improved.