-
-
Notifications
You must be signed in to change notification settings - Fork 510
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
Allow copying algebraic closure of finite field #38994
base: develop
Are you sure you want to change the base?
Allow copying algebraic closure of finite field #38994
Conversation
Documentation preview for this PR (built with commit 989037c; changes) is ready! 🎉 |
…ebraic-closure-copy
@nbruin ? I don't know about copying |
I'm not sure what the copying behaviour should be for this structure. Reading the intro of the file, there is a very particular reason why the algebraic closure is not UniqueRepresentation. It is EqualityById, which looks like it's not a problem.
suggests that indirectly there IS a preferred copy: the one cached on the corresponding prime field. The fact that multiple copies can exist would suggest that at least deep_copy should be able to produce a separate copy. Perhaps check with Peter Bruin @pjbruin ? At the moment, only prime finite fields have algebraic closures. As far as I can see, prime finite fields are not formally UniqueRepresentation, but producing a finite field from More particularly:
and I think the following is actually problematic
|
If I understood correctly, the problem is we cannot really fix pickle, otherwise if we dump an element to the disk, exit Sage, reopen it (and thus the computed pseudo-conway_polynomial is different), then there's no consistent way to convert the unpickled element to an element of the (new) Unless… we modify pseudo-Conway polynomial code to be deterministic, this is backwards compatible (we never promise it is random!). Still, unpickling elements from previous SageMath's versions won't work — we again need a deprecation period. Then there are e.g. https://arxiv.org/abs/2107.02257 that allows a different construction — but if I understood correctly
See also #38376 |
Choosing pseudo-conway polynomials in a deterministic way would probably allow solving the pickling problems. I don't immediately see why that wouldn't work for existing pickles (do those pickles store the computed Conway polynomials?), but pickle has a "version" system that allows you to recognize old pickles, that you can then reconstruct using a legacy method that you need to register somewhere. Changing to another implementation can be done with deprecation: just write and install the new code under other names, have a deprecation period where people get a warning if they use the old routines, pushing them to the new (so that would be Or if you feel that the benefits of the change are large enough to inconvenience some hypothetical users that depend on conwayness then just make the change. But perhaps check beforehand if any of them are vocal. |
Actually… if pseudo-Conway polynomials are really randomly chosen, then finite fields with degree greater than 1 cannot be UniqueRepresentation either. Yet: sage: deepcopy(GF(5^2))
Finite Field in z2 of size 5^2
sage: deepcopy(GF(5^2)) is GF(5^2)
True
sage: loads(dumps(GF(5^2))) is GF(5^2)
True I think modifying pickle return the algebraic closure itself would be just fine (in practice). Either that or we should modify the pickle behavior for non-prime finite fields. |
…ebraic-closure-copy
I can think of three "obvious" ways to copy
I guess it would be fine for |
…ebraic-closure-copy
…ebraic-closure-copy
Fixes #38988
I choose to make the copy return itself, because that's what
GF(p)
does at the moment anyway, and the field is immutable.Not sure how much of a hack for me to implement
__copy__
and__deepcopy__
instead of__reduce__
, but pickling and unpickling at the moment works well anyway (although it returns a different instance)📝 Checklist
⌛ Dependencies