-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add test for NiceMonomorphismByDomain #38
Open
fingolfin
wants to merge
1
commit into
master
Choose a base branch
from
mh/NiceMonomorphismByDomain
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# | ||
gap> START_TEST("misc.tst"); | ||
|
||
# test NiceMonomorphism for subgeometries, which uses hash tables | ||
gap> pg := PG(1,5^4); | ||
ProjectiveSpace(1, 625) | ||
gap> sub := CanonicalSubgeometryOfProjectiveSpace(pg, GF(5)); | ||
Subgeometry PG(1, 5) of ProjectiveSpace(1, 625) | ||
gap> g := CollineationGroup(sub); | ||
The FinInG collineation group PGL(2,5) of Subgeometry PG(1, 5) of ProjectiveSpace(1, 625) | ||
gap> Size(g); | ||
480 | ||
gap> Number([1..10], i -> PseudoRandom(g) in g); | ||
10 | ||
|
||
# | ||
gap> STOP_TEST("misc.tst", 10000 ); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dug a bit and this is the problem: the test
elm in g
above is implemented by this method:Now everything works fine until the final check
PreImagesRepresentative( nice, img ) = elm;
. Here we observe this:So, yeah: those preimages are not equal. The differ on
pg
-- but they are equal when restricted to the subspacesub
, which is only defined over GF(5) after all. So those two induce a monomorphism there after all.So
CollineationGroup(sub);
returns a group generated by collineations of the parent geometry. But two such collineations ofpg
, when restricted tosub
, might induce identical maps (e.g. the identity onpg
and the Frobenius automorphismon
pgboth induce the identity on
sub`).Based on this, I think the real problem is that the Frobenius of GF(5^4) is used as a generator for the subgeometry. I'll see if I can fix that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some discussion mentioning
CollineationGroup
and issues with nice monomorphisms in the package manual, but I don't understand what it says... sighThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon re-reading a few times, I think I get it now: applied to our example, where the subgeometry$PG(1,GF(5^4))$ is isomorphic to $PG(1,GF(5))$ , it says that the collineation group of $PG(1,GF(5))$ ?
sub
ofsub
is defined to be a certain conjugate of the collineation group ofI think this is meant to deal with subgeometries isomorphic to$PG(1,GF(5))$ but different from the "canonical" (with respect to the standard frame) copy of $PG(1,GF(5))$ . But in the example here, this is already the group in question.
So more is going on: indeed, the collineation group here is$PGL(2,5)$ , isn't it, of size 120. But the collineation group the code produces has size 480. Indeed, what we actually get is the collineation group we are looking for but extended by the Frobenius of the large field $GF(5^4)$ , which has order 4.
If we just omit that generator, we actually get the "right" group. But that won't work for e.g.$PG(1,GF(5^2))$ , because there we need a Frobenius of $GF(5^2)$ ; but that just isn't an element of the "big" group $P\Gamma(2,GF(5^4))$ -- indeed I don't think $P\Gamma(1,GF(5^2))$ is a subgroup of $P\Gamma(2,GF(5^4))$ (at least not in a "natural" way). That's because the Frobenius $f_{5^2}$ of $GF(5^2)$ has two "lifts" to $GF(5^4)$ , both of order 4, namely $f_{5^4}$ and $f_{5^2}^3=f_{5^2}^{-1}$ ... the group generated by $f_{5^2}$ is a quotient of $f_{5^4}$ but the quotient map has no section...
OK, so maybe this is intentional, and the collineation group of the subgeometry is meant to be given "extended by the Frobenius of the parent geometry" ? But that really should be in the documentation then? (Maybe it is, but then I didn't find it, and would appreciate pointers as to where I can read more).
If the intent is to use this "extended" collineation group, then a possible fix for the issue here would be to tweak
NiceMonomorphismByDomain
to extend the group action to also consider the full "Frobenius" part separately (what I mean is this: right now we use the action onAsList(Points(geom))
. We could add to this the orbit of a primitive element of the large field under the action of just the Frobenius part of each collineation (i.e.collineation!.frob
).Alternatively, a bigger change would be to drop the illusion that the collineation group of the subgeometry is a subgroup of the collineation group of the parent geometry. This is wrong in this example here and also wrong in general, so I am not sure why the code seemingly attempts to pretend otherwise?