support Oscar.iso_gap_oscar for Alnuth's FieldByMatrices
#2254
Conversation
src/GAP/iso_gap_oscar.jl
Outdated
|
|
||
| # Construct the isomorphic number field in Oscar. | ||
| cfs = GAPWrap.CoefficientsOfUnivariatePolynomial(pol) | ||
| R, x = polynomial_ring(QQ, "x") |
There was a problem hiding this comment.
| R, x = polynomial_ring(QQ, "x") | |
| R, x = polynomial_ring(QQ, "x", cached = false) |
There was a problem hiding this comment.
Why shall we force the creation of a new polynomial ring whenever a new number field in Oscar gets created?
There was a problem hiding this comment.
I am happy if you want to keep it, but why shall we force a useless dictionary lookup whenever a new number field is created?
There was a problem hiding this comment.
Do we have a written down policy when to enable the caching and when not? @fieker?
src/GAP/iso_gap_oscar.jl
Outdated
| cfs = GAPWrap.CoefficientsOfUnivariatePolynomial(pol) | ||
| R, x = polynomial_ring(QQ, "x") | ||
| polFO = R(Vector{QQFieldElem}(cfs)) | ||
| FO, _ = number_field(polFO, "z") |
There was a problem hiding this comment.
| FO, _ = number_field(polFO, "z") | |
| FO, _ = number_field(polFO, "z", cached = false) |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2254 +/- ##
===========================================
+ Coverage 0.00% 72.48% +72.48%
===========================================
Files 372 373 +1
Lines 51949 51793 -156
===========================================
+ Hits 0 37544 +37544
+ Misses 51949 14249 -37700
|
- Call `number_field` with option `cached = false`. (The field will be cached in the map created by `iso_gap_oscar`.) - Use `Hecke.Globals.Qx` for creating number fields. - Call `polynomial_ring` over other base rings with option `cached = false`. (The polynomial ring will be cached in the map created by `iso_gap_oscar`.)
The idea is to get a better interface between Oscar and those functions from Alnuth which used to delegate tasks to Pari/GP, see #2220 for the current situation.
Note that the proposed
Oscar.iso_gap_oscarand the already availableOscar.iso_gap_oscarcode that deals with algebraic extensions on the GAP side cannot be used to simply replace the_OscarFieldthat has been introduced in #2220, because of the difference betweenDefiningPolynomialandIntegerDefiningPolynomial(see my comment in the discussion of #2220).