Skip to content

Daniels constructive recognition code - #372

Draft
fingolfin wants to merge 6 commits into
masterfrom
DanielsConstructiveRecognitionCode
Draft

fingolfin wants to merge 6 commits into
masterfrom
DanielsConstructiveRecognitionCode

Conversation

@fingolfin

Copy link
Copy Markdown
Member

The rest of the constructive recognition code written by @danielrademacher for his thesis.

The code in this PR has diverged from PR #371, and will need to be re-integrated with that, and of course master.

Note that I decided to discard the history from Daniel's repository; first off, it would have been a major pain to migrate it (the repository was not a fork of the recog repo, but rather a fresh repository that imported some version of the recog code, but I was not able to determine which exactly). Secondly, I don't think it is useful for the public to see what Daniel tried etc. over time.

I would also like to emphasize that Daniel never got around to "polishing" this code. There are some functions which "obviously" could be implemented much better, but that's not because Daniel wasn't aware, but because he was very good in deciding to not waste time on optimizing stuff while writing a prototype. Just write the first thing that comes to mind, and if it all comes together in the end, sit down and do it properly.

Anyway: I am not sure we'll ever merge this PR as-is, it may rather end up as a "quarry" from which we take code and get it merged in separate PRs.

@fingolfin

Copy link
Copy Markdown
Member Author

Superseded by #525, which stacks this code on #524 (the SL half) and #523 (a
pure split of gap/projective/sl.gi). Leaving this open for reference.

Not rebased: the SL files here are the ancestors of #371's, so #525 drops them
and takes only the Sp/SU/O half. That works because nothing in Sp/SU/O depends
on anything #371 changed — the only cross-call into the SL code is
RECOG.FindStdGensUsingBSGS, which is in classicalnatural.gi on master
anyway.

Things found while rebuilding, recorded here:

read.g still reads gap/projective/sl.gi after the new SL files, so on
this branch the entire new SL layer — FindStdGens_SL, SLn_UpStep,
SLn_constructsl2 and the rest — is shadowed dead code.

gap/projective/naming/ClassicalNamingAndUtils.gi breaks
RecogniseClassical.
It defines an unqualified global HasElementsMultipleOf
which overrides the one in gap/matrix/classical.gi (read earlier) with a
function returning Success/NeverApplicable — which are strings — instead
of a boolean. classical.gi calls it as if not HasElementsMultipleOf(...) in
about twenty places. The file is otherwise a hand-fork of classical.gi with
zero callers, so #525 leaves it out entirely; it needs its own PR.

tst/working/slow/ConstructiveRecognition.tst would run on every CI job
(tst/testall.g recurses into tst/working/) and cannot pass:
while i < 20 and res = fail do ... od never increments i, so any failure is
an infinite loop; the SU block passes an unbound k to RECOG.MakeSU_StdGens,
whose signature is (d,q,p,ext,phi); and START_TEST names it bugfix.tst.
#525 fixes those and parks it in tst/broken/veryslow/, because the code behind
it does not work yet: FindStdGens_Sp errors on Sp(10,3), hangs on Sp(6,3),
and one branch of FindStdGens_Sp2 still ends in
Error("here"); return "TODO";. We found no input at all for which
FindStdGens_Orthogonal works.

Smaller ones, all fixed in #525: RECOG.CentralOrder and
RECOG.GenerateStatictsSU are each defined twice (the latter's second copy is
really the Sp variant under a forgotten name); RECOG.RandomElementOfOrder
clashes between sl2_BlackBox.gi and blackboxClassical.gi with incompatible
arities; and BilinearForm/SymplecticForm/SymmetricBilinearForm/QuadraticForm
are each defined twice with identical bodies.

Your note that this may end up "a quarry rather than a PR" still looks right —
#525 is that quarry, but stacked and loading cleanly, with tst/testall.g green.

🤖 Generated with Claude Code

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