Skip to content

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented Oct 4, 2025

Previously libgap(QQ[I]) fails. Now it works.

also add some minor clean-ups and documentation changes.

The pointed out bug (that is, Sage FiniteField ↔ GAP GF does not keep the DefiningPolynomial) is to be addressed in a subsequent pull request.

context: I was trying to improve the performance of GF(next_prime(2^128)^4) by using GAP's StandardFF package, see #38376 (comment). but at the moment the conversion between Sage and Gap FiniteField is wrong.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Oct 4, 2025

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

a
Check that libgap global variables does not interfere with this method
(as it should if the method is correctly implemented and avoid ``libgap.eval``)::
Copy link
Contributor

@fchapoton fchapoton Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does --> do
avoid ---> avoids

sage: z = QQ['z'].0
sage: K.<zeta> = NumberField(z^2 - 2)
sage: K._gap_init_() # the following variable name $sage1 represents the F.base_ring() in gap and is somehow random
sage: K._gap_init_() # random (the variable name $sage1 represents F.base_ring() in gap is somehow random)
Copy link
Contributor

@fchapoton fchapoton Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather write "and is random" so that the sentence is correct

raise NotImplementedError("Currently, only simple algebraic extensions are implemented in libgap")

from sage.libs.gap.libgap import libgap
q = self.polynomial()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q is not used ; you could also merge the last 2 lines

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.

2 participants