Skip to content
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

convenience methods for field embeddings #38487

Merged

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Aug 8, 2024

Currently, the best(?) way to compute embeddings between fields is somewhat hidden inside the Hom object. In this patch we add helper methods to access such embeddings. In addition, we add an .algebraic_closure() method to non-prime finite fields.

These seem useful.

@grhkm21
Copy link
Contributor

grhkm21 commented Aug 9, 2024

One minor (relevant) error with tests/docs/CI.

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 10, 2024

Thanks, should be fixed.

@yyyyx4 yyyyx4 force-pushed the public/convenience_methods_for_field_embeddings branch from b3429d2 to 1e82473 Compare August 10, 2024 21:57
@GiacomoPope
Copy link
Contributor

Tiny comment about documentation, otherwise looks good to me.

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 21, 2024

Indeed, thanks, done.

@vbraun
Copy link
Member

vbraun commented Aug 26, 2024

Test fail

yyyyx4 and others added 6 commits October 20, 2024 09:32
Co-authored-by: Giacomo Pope <[email protected]>
…nical

...hence the output here would be more or less meaningless. We should
fail instead, until algebraic closures of non-prime finite fields are
supported "properly".

(reverts part of 87e912b)
@yyyyx4 yyyyx4 force-pushed the public/convenience_methods_for_field_embeddings branch from 61fe1af to a2fdcc9 Compare October 21, 2024 16:19
@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 21, 2024

Thanks for the review, @S17A05! I think I've addressed all your comments.

Copy link

Documentation preview for this PR (built with commit a2fdcc9; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@S17A05
Copy link
Member

S17A05 commented Oct 22, 2024

Thanks for the review, @S17A05! I think I've addressed all your comments.

Thanks for the changes, looks good to me now! (For the record: The timeout in the long [p-z] tests is unrelated)

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 23, 2024
    
Currently, the best(?) way to compute embeddings between fields is
somewhat hidden inside the `Hom` object. In this patch we add helper
methods to access such embeddings. In addition, we add an
`.algebraic_closure()` method to non-prime finite fields.

These seem useful.
    
URL: sagemath#38487
Reported by: Lorenz Panny
Reviewer(s): Giacomo Pope, Sebastian A. Spindler
@vbraun vbraun merged commit 5c25c12 into sagemath:develop Oct 26, 2024
17 of 19 checks passed
@yyyyx4 yyyyx4 deleted the public/convenience_methods_for_field_embeddings branch October 27, 2024 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants