Opened 5 years ago
Closed 5 years ago
#20746 closed enhancement (fixed)
Pushout for real embedded number fields
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  number fields  Keywords:  days74, days84 
Cc:  jdemeyer, tscrim  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  4a52745 (Commits, GitHub, GitLab)  Commit:  4a52745b734543ca8e65f2ac3050c9a73f223edd 
Dependencies:  Stopgaps: 
Description (last modified by )
Implement the following pushout
sage: K.<a> = NumberField(x^2  3, embedding=AA(3)**(1/2)) sage: L.<b> = NumberField(x^2  2, embedding=AA(2)**(1/2)) sage: a+b 3.146264369941973? sage: parent(_) Algebraic Real Field sage: a < b False
Change History (12)
comment:1 Changed 5 years ago by
 Branch set to u/vdelecroix/20746
 Commit set to 4a52745b734543ca8e65f2ac3050c9a73f223edd
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
 Description modified (diff)
comment:3 Changed 5 years ago by
 Cc tscrim added
comment:4 Changed 5 years ago by
 Status changed from needs_review to needs_work
Isn't it a bit drastic to declare AA
to be the pushout rather than to create a suitable number field? I think a better solution would be to use the existing composite_fields()
method, which respects embeddings:

src/sage/rings/number_field/number_field_base.pyx
a b cdef class NumberField(Field): 350 350 else: 351 351 raise ValueError("No embedding set. You need to specify a a real embedding.") 352 352 353 def _pushout_(self, other): 354 if (isinstance(other, NumberField) and self._embedded_real and 355 (<NumberField>other)._embedded_real): 356 return self.composite_fields(other)[0]
This makes the examples in the current branch work just as well (you can see the numerical values after coercing into AA
).
comment:5 Changed 5 years ago by
Fair enough. Though it is more expensive.
comment:6 Changed 5 years ago by
And this is order dependent
sage: K.<a> = NumberField(x^2  3, embedding=AA(3)**(1/2)) sage: L.<b> = NumberField(x^2  2, embedding=AA(2)**(1/2)) sage: a+b ab^3  10*ab
versus
sage: K.<a> = NumberField(x^2  3, embedding=AA(3)**(1/2)) sage: L.<b> = NumberField(x^2  2, embedding=AA(2)**(1/2)) sage: b+a ba^3 + 10*ba
comment:7 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 5 years ago by
This coercion has its advantages:
 makes it possible to do arithmetic operations with elements with two different
NumberFields
as parents. Right now returns a TypeError?:
sage: J = NumberField(x^2  2,'s') sage: s = J.gens()[0] sage: K = NumberField(x^3  2,'t') sage: t = K.gens()[0] sage: s * t Traceback (most recent call last): ... TypeError: unsupported operand parent(s) for *: ...
 It is lazy, it does have to compute the join.
Depending on the usage, one may want to get the join of the fields using composite_field
, this introduces another variable and is order dependant. I guess that if one wants to do computations on the long run with that field, one would already use the right field from start.
IMHO, this ticket already fixes an operation that one would naturally want to have at hand. So, I would make this a positive review.
comment:9 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:10 Changed 5 years ago by
 Reviewers set to JeanPhilippe Labbé
comment:11 Changed 5 years ago by
 Keywords days84 added
comment:12 Changed 5 years ago by
 Branch changed from u/vdelecroix/20746 to 4a52745b734543ca8e65f2ac3050c9a73f223edd
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac 20746: pushout for real embedded number fields