Improve and unify names

[ssiccha]

RecognitionInfoFamily -> RecogNodeFamily
IsRecognitionInfo -> IsRecogNode
RIFac -> ImageRecogNode
RIKer -> KernelRecogNode

PR 1 for #177

[ssiccha]

Merging this may clash with #176. I'll try whether I can rebase that branch easily on top of this one. Otherwise we'd have to wait until #176 is merged first.

[ssiccha]

@FriedrichRober The tests fail because we rename e.g. RIFac but not Has... and SetRIFac. We need to update our script accordingly.

[wilfwilson]

In general I approve, while also supporting Max's suggestion.

This is perhaps an ignorant question (I've not really thought about recog for quite a while) - is naming this in terms of 'image' definitely more appropriate than naming it in terms of 'factor'? I only ask because (I presume) the old name RIFac was referring to a 'factor'; although the actual documentation talked about the 'image'.

[ssiccha]

because (I presume) the old name RIFac was referring to a 'factor'; although the actual documentation talked about the 'image'.

@wilfwilson The way I understood the name is that if a node in our tree computes a reduction homomorphism, then the image of said homomorphism is the factor of its source by its kernel. I find using image a lot more natural here.

[ssiccha]

Ah wait, I just understood why previously factor was used. In magma, the package to recognize matrix groups constructively is called CompositionTree. The computed tree can be thought of as a generalization of a composition series. In that sense the image of a homomorphism is a factor of said tree.

[wilfwilson]

Cool 🙂 I'm happy for it to be called image, but as just curious.

[ssiccha]

Rebased onto master, fixed the tests and the documentation. Please in particular have a look at the file doc/renaming.xml or Chapter 8 "Renaming".

[codecov]

Codecov Report

Merging #253 (d691e2e) into master (30b7537) will not change coverage.
The diff coverage is 83.67%.

❗ Current head d691e2e differs from pull request most recent head fd91087. Consider uploading reports for the commit fd91087 to get more accurate results

@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   77.86%   77.86%           
=======================================
  Files          43       43           
  Lines       18389    18389           
=======================================
  Hits        14319    14319           
  Misses       4070     4070           

Impacted Files	Coverage Δ
gap/generic/SnAnUnknownDegree.gi	97.74% <ø> (ø)
gap/matrix/matimpr.gi	69.03% <ø> (ø)
gap/perm.gi	97.27% <ø> (ø)
gap/projective/d247.gi	74.92% <ø> (ø)
gap/base/recognition.gi	68.79% <70.37%> (ø)
gap/base/recognition.gd	100.00% <100.00%> (ø)
gap/generic/KnownNilpotent.gi	97.18% <100.00%> (ø)
gap/generic/kernel.gi	89.87% <100.00%> (ø)
gap/matrix.gi	94.19% <100.00%> (ø)
gap/projective.gi	83.09% <100.00%> (ø)

[FriedrichRober]

I think the names are now very descriptive and the renamings are well documented.

[fingolfin]

Regarding factor vs. image: of course both make sense. However, we do talk about the "factor group" in many comments and the manual. There also still the attributes methodsforfactor or forfactor, which now don't match anymore. Of course we could also rename them.

Taking a step back, I guess both names make sense: a factor group is the image of a group homomorphism... However, my mind has a slight tendency to think of this as a "proper factor", i.e. the homomorphism has a non-trivial kernel, while in our case, there are also sometimes isomorphisms... While "image" does not have such connotation to me. So maybe that means "image" would be better indeed, at least if other people suffer from the same prejudice regarding "factor" as I do :-)

[ssiccha]

Hm, interesting. When you think of an inner node in the recog tree as a homomorphism it is IMO more direct to talk of kernels and images. When you think of an inner node as a group, it is IMO more direct to talk of normal subgroups and factors. Of course inner nodes have both a group and a homomorphism associated (leaf nodes also have an isomorphism into a standard copy, but that's another topic). At the moment though we're mixing both by talking about kernels and factors.

And yes, when I hear or read "factor" I also don't think of a factor by the trivial subgroup. :D

[ssiccha]

Two follow up PRs would be:

• turn Info into Node where sensible
• turn Factor into Image where sensible

In both cases we should also search the manual for left over occurences of Info and Factor. I can use my tags file to search for all names which contain Info or Factor.

[wilfwilson]

I've enjoyed this discussion 🙂

[ssiccha]

I've rebased this onto master and squashed the "fixup" commits. No other changes were introduced.

[ssiccha]

I'll try to get #176 into a state where it can be merged into master, i.e. such that the tests pass etc. Then I'd merge in this PR (#253) and then @FriedrichRober and I can continue work on #176 in a different branch and PR.

[ssiccha]

Rebased onto master. Now I need to change factor in the manual to image and make sure that updating the index is also done in the script @FriedrichRober and I wrote.

[ssiccha]

I've replaced most occurences of factor with image, see the fixup! commit. Of course, where factor meant a prime factor or the factor of a polynomial I left it alone. :)

[ssiccha]

@fingolfin Could you have another look?

[ssiccha]

Ok, I got rid of almost all appearances of '\<info\>'. The remaining ones don't have anything to do with recog info records.

In a future PR I think we could rename EmptyRecognitionInfoRecord to RecogNode and document it.

[wilfwilson]

LGTM.

[ssiccha]

bugfix.tst failed because now line breaks are different.