Skip to content

Conversation

@AnnaKDS
Copy link
Collaborator

@AnnaKDS AnnaKDS commented Dec 11, 2025

I am working on improving the documentation to include a chapter that is particularly directed to people who want to recognise groups with this package and don't have much theoretical background.

It's still WIP but if you have any suggestions, please feel free to comment.

@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.12%. Comparing base (6d84867) to head (cecdf1f).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
- Coverage   78.55%   69.12%   -9.43%     
==========================================
  Files          43       43              
  Lines       18281    18321      +40     
==========================================
- Hits        14361    12665    -1696     
- Misses       3920     5656    +1736     

see 18 files with indirect coverage changes

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

<Heading>Examples</Heading>

In order to understand the output we consider some examples. Note that many
methods used to compute a recognition tree a randomised algorithms. This means
Copy link
Contributor

@aniemeyer aniemeyer Dec 11, 2025

Choose a reason for hiding this comment

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

change a randomised to are randomised

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
methods used to compute a recognition tree a randomised algorithms. This means
methods used to compute a recognition tree are randomised. This means

or

Suggested change
methods used to compute a recognition tree a randomised algorithms. This means
methods used to compute a recognition tree are using randomised algorithms. This means

factor of <M>H</M> (abbreviated with <Q>F</Q> which stands for Factor) and a normal
subgroup of <M>H</M> (abbreviated with <Q>K</Q> which stands for Kernel). <P/>

In this example the root node presents the input group <M>SL(10,5)</M>. Starting
Copy link
Contributor

Choose a reason for hiding this comment

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

presents -> represents (note: represents is also used below)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In this example the root node presents the input group <M>SL(10,5)</M>. Starting
In this example the root node represents the input group <M>SL(10,5)</M>. Starting

H</M>, and then recursively analysing both the kernel
<M>\ker(\varphi) = N \unlhd G</M> and the image
im<M>(\varphi) \cong G/N</M>. This continues until the remaining groups
are (quasi-)simple, which are the fundamental building blocks of all
Copy link
Contributor

Choose a reason for hiding this comment

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

We might also allow almost simple, e.g. we treat S_n as a leaf

<recognition node MovesOnlySmallPoints Size=3628800>]]>
</Log>

This recognition tree only consists of the root node representing the input
Copy link
Contributor

Choose a reason for hiding this comment

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

Here `root node' is mentioned. Should it be stated above that RecogniseGroup always returns the root node?

</Log>

Note that the first few lines are not part of the returned recognition node. You
can obtain these further information by setting the InfoLevel accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

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

replace these' by such'

The setup is described in <Cite Key="NS06"/>. <P/>

The framework allows to build composition trees and handles the builtup
This package is about group recognition.<P/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this statement is a bit misleading. Really, this package is about computing a data structure to enable the user to answer questions about a given group. The actual `recognition' as described in the paragraph below really occurs at the leaf nodes and not necessarily for the input group. Maybe we could say: This package is about building a data structure for a group which encodes a composition tree and the recognition of groups closely related to the composition factors?

"user.xml",
"recognition.xml",
"methsel.xml",
"afterrecog.xml",

Choose a reason for hiding this comment

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

If this chapter shall be left out then also the cross-reference in Section "Overview over this manual" must be removed (label "afterrecog").


<#Include Label="RecognisePermGroup">
<#Include Label="RecogniseMatrixGroup">
<#Include Label="RecogniseProjectiveGroup">

Choose a reason for hiding this comment

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

Currently the four Recognize*Group functions are included in two chapters, GAPDoc prints warnings about that,
I suggest to include RecognizeGroup here and to include the other three functions in Chapter "Group recognition".

Chapter <Ref Chap="install"/> describes the installation of this package.<P/>

Chapter <Ref Chap="user"/> presents a user-friendly introduction to this
package.<P/>

Choose a reason for hiding this comment

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

Perhaps emphasize that examples are shown in the Section "Examples".
(And remove the now outdated reference at the end of the "Overview" section ("Finally, Chapter ... shows some instructive examples ...").

@ThomasBreuer
Copy link

some general remarks:

  • The introduction distinguishes "naming" and "constructive recognition".
    Can a user ask just for the result of the "naming" phase, or is the result of the user functions always that of the "constructive recognition"? In the latter case, the distinction of the two phases belongs to a more technical part of the manual.
  • Chapter "Group recognition" distinguishes a "recognition phase" and a "verification phase".
    What is the status of the results of RecognizeGroup: Is the return value always verified, or may it be incorrect? Asked the other way round: Can one ask for a result without verification?


In order to work with group elements efficiently and constructively throughout
this process,
<Package>recog</Package> uses <Q>Straight-Line Programs</Q> (or <Q>SLPs</Q>). An <Q>SLP</Q> is a

Choose a reason for hiding this comment

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

Perhaps add a cross-reference to the section on straight line programs in the GAP Reference Manual.


Chapter <Ref Chap="recognition"/> describes the generic, recursive procedure
used for group recognition throughout this package. At the heart of this
procedure is the definition of <Q>FindHomomorphism</Q> methods, which is also

This comment was marked as duplicate.

@fingolfin
Copy link
Member

@ThomasBreuer currently verification is not implemented. But it definitely should be made possible for the user to control whether verification happens; and also perhaps certain probabilities.

Copy link
Member

@frankluebeck frankluebeck left a comment

Choose a reason for hiding this comment

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

This is a welcome improvement of the recog documentation.

I have added a few more comments.

After addressing the current comments this should be merged soon. Of course, it can be further improved any time later.

@fingolfin
Copy link
Member

I agree with @frankluebeck: I'd rather merge this sooner than later. Even if there are still TODO comment in there. Once it is merged, follow-up PRs can further improve things.

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.

5 participants