-
Notifications
You must be signed in to change notification settings - Fork 162
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
Extend meataxe to be able to detect invariant forms of reducible modules (just one possible form is returned) #5803
base: master
Are you sure you want to change the base?
Conversation
ae7ea2b
to
8cb1588
Compare
A main change seems a change from IsomorphismIrred to IsomorphismModules. What I want to make sure is that there will not be a chance of running in infinite recursion. I have not looked again into the code of these two functions, but the former is a straightforward spinning algorithm, while the second is a rather elaborate calculation that uses a lot of other MeatAxe routines. |
Calls to
So nothing outside of
So all in all I don't see how this could cause a problem? |
0829bf4
to
e7eb173
Compare
lib/meataxe.gi
Outdated
# if l mod (r-1) <> 0 then | ||
# Error("Form does not seem to be of the right kind (not (q-1)st root)!"); | ||
# fi; | ||
# iso:=Z(q)^(l/(r-1)) * iso; |
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.
@ThomasBreuer @hulpke @jandebeule any ideas what the commented code above (inside SMTX.InvariantSesquilinearForm
) is meant to be good for?
It looks to me like an attempt to "normalize" the isomorphism (and hence invariant form) we compute here). But the form can in fact be scaled by an arbitrary scalar (not just by a norm), so I don't understand why it we choose this specific one.
The reason I stumbled over this, and subsequently commented it out (to be deleted) is that it utterly fails if the given module is e.g. the sum of two isomorphic modules - then isot
above in general won't be diagonal at all (test cases for that added below). On the other hand I can't really think of any reason why it would be useful. But I am sure whoever added this had a good reason, so I wonder what I am missing.
Any suggestions would be highly welcome!
5f9e3d4
to
d691409
Compare
@ThomasBreuer @hulpke @jandebeule any thoughts on this PR resp. my question above? |
No, I do not understand why this scaling happens, and which code may rely on it. If we decide that this piece of code should get removed then also the line |
This got introduced after 4.B.1 and before 4.4. |
... by deleting unnecessary 'normalization' (???) code.
d691409
to
3147c2b
Compare
I have a private git repository with the full commit history, and tracked the origin of the comment "Replace iso by a scalar multiple" to a commit made 2003-01-06 10:05:29 by @stevelinton with commit message "Derek Holt's code for Invariant forms." So perhaps I should ask Derek about it... |
Co-authored-by: James Mitchell <[email protected]>
# Replace iso by a scalar multiple to get iso twisted symmetric | ||
q:=Size(module.field); | ||
r:=RootInt(q,2); | ||
isot:=List( TransposedMat(iso), x -> List(x, y->y^r) ); | ||
isot:=iso * isot^-1; |
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.
Looking at this again I realize that of course this code is needed. Not sure why I was too dense to grasp it before: the reason why is right in this comment: to ensure we get a form which is twisted symmetric. I.e. the matrix M
representing the isomorphism aka the form should satisfy
Indeed in the code the matrix is called iso
and that's it computes iso * isot^-1
. If this is the identity, we are done. If it is a scalar matrix, we can hopefully scale M
to satisfy the property, which is what the below does.
I'll restored the deleted code and add comments with details.
That then gets me back to the tests failing then, because now in the reducible case, iso * isot^-1
is not always diagonal. I did not yet look into the details, but surely this happens if there is a homogeneous reducible summand of the module, i.e.,
fi; | ||
if IsBound(module.InvariantBilinearForm) then | ||
return module.InvariantBilinearForm; | ||
fi; | ||
DM:=SMTX.DualModule(module); | ||
iso:=MTX.IsomorphismIrred(module,DM); | ||
iso:=MTX.IsomorphismModules(module,DM); | ||
if iso = fail then |
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.
Here, too, we might get additional problems, i.e., iso
may not be symmetric -- we ought to add a check for that.
No description provided.