Opened 6 years ago
Closed 6 years ago
#18740 closed enhancement (fixed)
Reduce overhead for relative number field elements
Reported by:  pbruin  Owned by:  

Priority:  minor  Milestone:  sage6.9 
Component:  number fields  Keywords:  relative number field pari 
Cc:  Merged in:  
Authors:  Peter Bruin  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  e59e8ec (Commits, GitHub, GitLab)  Commit:  e59e8ec7b0ac671167d7df8acd00631cb1fcdc76 
Dependencies:  #18727, #18739  Stopgaps: 
Description
We should use the PARI functions wrapped in #18739 to avoid initialisation of PARI rnf
structures when only doing simple arithmetic in relative number fields.
Summary of changes:
 new method
NumberFieldElement._pari_polynomial
;  replace the method
_pari_rnfequation
ofNumberField_relative
by_pari_rnfeq
, which calls the PARI functionnf_rnfeq
(which returns slightly more information thanrnfequation
);  replace
K.pari_rnf().rnfeltreltoabs(x)
byK._pari_rnfeq().eltreltoabs(x)
and similar;  remove the misleadingly named (and now obsolete) method
NumberField_relative._rnfeltreltoabs
, which only did something meaningful for elements in the base field;  merge
NumberField_relative.pari_polynomial
intoNumberField_generic.pari_polynomial
.
Change History (11)
comment:1 Changed 6 years ago by
 Branch set to u/pbruin/18740relative_number_fields
 Commit set to cddcaafd5aa1d6cdc860a3624ca247283eed985a
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Status changed from needs_review to needs_work
 Work issues set to method names changed in #18739
comment:3 Changed 6 years ago by
 Commit changed from cddcaafd5aa1d6cdc860a3624ca247283eed985a to e6dd7a1198fe621fcc3b9c84a20948db2a257307
comment:4 Changed 6 years ago by
 Commit changed from e6dd7a1198fe621fcc3b9c84a20948db2a257307 to c661096b45e80a7227eba537ae1f7dc9e3836c1d
comment:5 Changed 6 years ago by
 Status changed from needs_work to needs_review
 Work issues method names changed in #18739 deleted
comment:6 followup: ↓ 7 Changed 6 years ago by
 Milestone changed from sage6.8 to sage6.9
 Status changed from needs_review to needs_work
Hello Peter,
Instead of
f = pari(self._coefficients()).Polrev() return f.change_variable_name(name)
you can do
return pari(self._coefficients()).Polrev(name)
that would avoid a copy.
Why did you rename _rnf_equations
into _rnfeq
. I understand that relative_number_field_equations
is a bit long but the Sage standard is more toward no abbreviation at all. Is the name rnfeq
standard in PARI/GP?
Vincent
comment:7 in reply to: ↑ 6 ; followup: ↓ 10 Changed 6 years ago by
Salut Vincent,
Instead of
f = pari(self._coefficients()).Polrev() return f.change_variable_name(name)you can do
return pari(self._coefficients()).Polrev(name)that would avoid a copy.
OK, I will change this.
Why did you rename
_rnf_equations
into_rnfeq
. I understand thatrelative_number_field_equations
is a bit long but the Sage standard is more toward no abbreviation at all. Is the namernfeq
standard in PARI/GP?
The reason is that _pari_rnfequation()
was a wrapper around the PARI/GP function rnfequation()
, whereas _pari_rnfeq()
wraps the internal PARI library function nf_rnfeq()
. It is not just a change of name, but a different method returning similar (but more) output. I think it is best to keep the name of the Sage close to the PARI name. I didn't want to keep a separate method _pari_rnfequation()
because it was a private method which isn't used anymore.
comment:8 Changed 6 years ago by
 Commit changed from c661096b45e80a7227eba537ae1f7dc9e3836c1d to e59e8ec7b0ac671167d7df8acd00631cb1fcdc76
Branch pushed to git repo; I updated commit sha1. New commits:
e59e8ec  Trac 18740: speed up NumberFieldElement._pari_polynomial()

comment:9 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:10 in reply to: ↑ 7 Changed 6 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to positive_review
Replying to pbruin:
Why did you rename
_rnf_equations
into_rnfeq
. I understand thatrelative_number_field_equations
is a bit long but the Sage standard is more toward no abbreviation at all. Is the namernfeq
standard in PARI/GP?The reason is that
_pari_rnfequation()
was a wrapper around the PARI/GP functionrnfequation()
, whereas_pari_rnfeq()
wraps the internal PARI library functionnf_rnfeq()
. It is not just a change of name, but a different method returning similar (but more) output. I think it is best to keep the name of the Sage close to the PARI name. I didn't want to keep a separate method_pari_rnfequation()
because it was a private method which isn't used anymore.
I agree that your way is better.
comment:11 Changed 6 years ago by
 Branch changed from u/pbruin/18740relative_number_fields to e59e8ec7b0ac671167d7df8acd00631cb1fcdc76
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Merge branch 'u/pbruin/18739pari_rnf_conversion' into 6.8.b5
trac #18379 doc typo in pari/gen
trac #18739 raise into py3 syntax
Trac 18739: reviewer comments
Merge branch 'ticket/18739pari_rnf_conversion' into ticket/18740relative_number_fields
Trac 18740: update after renaming of methods in Trac 18739