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
Finite field homomorphisms and Frobenius endomorphisms #13214
Comments
comment:2
I guess your patch should also contain some *.pxd files since I receive the following error message.
|
comment:4
Xavier, if I define an element of GF(q2) which is in fact an element of GF(q), will this patch enable to convert it efficiently to the smaller field? If so, please can you tell me how? Paul |
comment:5
Oh sorry, I forgot *.pxd files. Thanks! Paul, the answer to your question is probably yes... at least if q is small enough so that you can work with givaro fields (otherwise, it won't be so efficient). Actually, I have not implemented automatic coercions between finite fields (essentially because embeddings between finite fields are not canonical). As a consequence it's not so easy to create embeddings. Nevertheless, here is what you can do:
If you are sure that you won't never use another embedding of K into L, you can even define f as a coerce map as follows:
Then the following will work:
|
comment:7
Please fill in your real name as Author. |
Author: Xavier Caruso |
comment:9
Xavier, I tried to apply the patch on top of Sage 5.1 but got:
Should I apply it on top of Sage 5.2 or later? Paul |
comment:10
please discard my last comment, I downloaded the patch in html form... Paul |
Work Issues: failing doctests |
Reviewer: Paul Zimmermann |
comment:11
on top of Sage 5.1 I get the following failures with this patch:
Paul |
comment:12
Dear Paul, I cannot reproduce the three last failures on my computer[*]. Could you please give me the complete output of 'sage -t'? [*] I'm currently working with sage 5.1.rc0; it's probably the reason. |
comment:13
These are the failures Paul observed (here on Sage 5.3) with this patch:
|
Attachment: trac_13214_hom_finite_field.2.patch.gz |
This comment has been minimized.
This comment has been minimized.
comment:14
I changed the coercion in sage/rings/finite_rings/homset.py. Now, this patch passes all doctests except for the one documented in sage/categories/modules_with_basis.py. I did not change the error messages there, because I wanted to discuss the failure first. |
This comment has been minimized.
This comment has been minimized.
comment:15
Thanks for catching this! I now understand why I didn't see this failure: it's just because this patch actually depends on #13184 and that the corresponding patch was applied on my computer. Sorry for this! I also attach a new version of my patch fixing the problem in modules_with_basis.py; I'm not really happy with my solution (but it has the advantage to be very short). Please let me now if you have some comment! |
Dependencies: #13184 |
Changed work issues from failing doctests to none |
comment:17
for the bot: Apply trac_13214_hom_finite_field.2.patch |
This comment has been minimized.
This comment has been minimized.
Changed author from Xavier Caruso to Xavier Caruso, Peter Bruin |
comment:34
In principle I am giving this a positive review, but since the reviewer patch I just uploaded contains some non-trivial changes, I think it is best if someone else reviews those changes first. Summary of changes in attachment: trac_13214-reviewer.patch:
|
Attachment: trac_13214-folded.patch.gz for convenience: unified patch combining trac_13214_hom_finite_field.patch and trac_13214-reviewer.patch |
comment:36
apply only trac_13214-folded.patch |
This comment has been minimized.
This comment has been minimized.
comment:38
I'm currently at Sage Days 53 and could spare some time on this one... unless Luca is nearing completion of his review? If I understand correctly, what needs to be done is to check Peter's changes as he already went over what Xavier did? |
comment:39
Replying to @jpflori:
I'm rather nearing completion of the new class I'm starting next week... so feel free to go ahead :) |
comment:40
Replying to @jpflori:
Yes, it would be nice if you could take a look either at my reviewer patch or at the combined one (the combined patch is only a little larger and probably easier to review). |
comment:41
patchbot: only Apply trac_13214-folded.patch |
Changed reviewer from Paul Zimmermann, Peter Bruin to Paul Zimmermann, Peter Bruin, Volker Braun |
comment:42
The review patch looks good to me. |
Merged: sage-5.13.beta2 |
Here is a patch implementing:
Apply: attachment: trac_13214-folded.patch
Depends on #8335
CC: @sagetrac-tfeulner @xcaruso @darijgr @jpflori @pjbruin @defeo @sagetrac-JCooley @sagetrac-dfesti
Component: finite rings
Keywords: frobenius finite fields sd51
Author: Xavier Caruso, Peter Bruin
Reviewer: Paul Zimmermann, Peter Bruin, Volker Braun
Merged: sage-5.13.beta2
Issue created by migration from https://trac.sagemath.org/ticket/13214
The text was updated successfully, but these errors were encountered: