Opened 5 years ago
Closed 5 years ago
#23647 closed defect (fixed)
wrong category for parent of composition of number field endomorphisms
Reported by:  Frédéric Chapoton  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  number fields  Keywords:  
Cc:  Travis Scrimshaw  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  768891b (Commits, GitHub, GitLab)  Commit:  768891be49e9a4b2409bc9b1bc731e306ac4e67d 
Dependencies:  Stopgaps: 
Description
namely
sage: K.<a, b> = NumberField([x^2  2, x^2  3]) sage: e, u, v, w = End(K) sage: e.abs_hom().parent().category() Category of homsets of number fields sage: u.abs_hom().parent().category() Category of homsets of number fields sage: v.abs_hom().parent().category() Category of homsets of number fields sage: (v*v).abs_hom().parent().category() Category of homsets of unital magmas and additive unital additive magmas
The last one should be the same as the other ones..
Change History (19)
comment:1 Changed 5 years ago by
Cc:  Travis Scrimshaw added 

comment:2 Changed 5 years ago by
So the problem comes down to this:
sage: v.parent() is End(K) True sage: v.parent() is Hom(K, K, category=NumberFields()) True sage: (v*v).parent() == End(K) True sage: Hom(K, K, category=Rings()) is End(K) # The issue False sage: Hom(K, K, category=Rings()) is (v*v).parent() True
The source of which, when you go down _composition_
, comes from this:
sage: v.category_for() # Which calls Category of rings sage: v.parent().homset_category() Category of rings
The latter should be NumberFields()
, not Rings()
because the subclass
sage.rings.number_field.morphism.NumberFieldHomset
does not pass NumberFields()
as its category. However, I tried passing the category of the domain, but this causes failures because ComplexLazyField
is not a number field and issues with CyclotomicField
that I don't understand.
comment:4 Changed 5 years ago by
No, I don't as it didn't work. However, I basically just added this to number_field/morphism.py
:
class NumberFieldHomset(RingHomset_generic): # This was already there def __init__(self, R, S, category=None): if category is None: from sage.categories.number_fields import NumberFields category = NumberFields() RingHomset_generic(self, R, S, category)
and the other variant I tried was
def __init__(self, R, S, category=None): if category is None: category = R.category() RingHomset_generic(self, R, S, category)
comment:5 Changed 5 years ago by
I have made another try, that causes interesting doctest failures:
Failed example: G = End(K); G Expected: Automorphism group of Cyclotomic Field of order 12 and degree 4 Got: Set of Homomorphisms from Cyclotomic Field of order 12 and degree 4 to Cyclotomic Field of order 12 and degree 4
EDIT: ok, ok. This is indeed a group.
comment:6 Changed 5 years ago by
Branch:  → public/23647 

Commit:  → ec7769501def864ad49a7282136cbc64e86be010 
comment:7 Changed 5 years ago by
in vanilla sage:
sage: End(QQ) Set of Homomorphisms from Rational Field to Rational Field sage: End(QuadraticField(7)) Automorphism group of Number Field in a with defining polynomial x^2  7 sage: End(CyclotomicField(7)) Automorphism group of Cyclotomic Field of order 7 and degree 6 sage: End(UniversalCyclotomicField()) Set of Homomorphisms from Universal Cyclotomic Field to Universal Cyclotomic Field sage: End(GF(5)) Automorphism group of Finite Field of size 5
and
comment:8 Changed 5 years ago by
This does work, but it is a very big hack IMO. I think it might be worth trying to do a more comprehensive fix. At least as a first step, we can put QQ in NumberFields
: #23761.
comment:9 Changed 5 years ago by
I believe UniversalCyclotomicField()
is not a number field as I believe is an infinitedimensional field extension of Q.
comment:10 Changed 5 years ago by
Branch:  public/23647 → public/23647v2 

Commit:  ec7769501def864ad49a7282136cbc64e86be010 → 56c31afa0082e075c5644c041ba4a30827e1434b 
comment:11 Changed 5 years ago by
Commit:  56c31afa0082e075c5644c041ba4a30827e1434b → 49ddc5d90892a2e7343539703f8aa2ce827aa16d 

Branch pushed to git repo; I updated commit sha1. New commits:
49ddc5d  trac 23647 fix doctest

comment:12 Changed 5 years ago by
This seems to work (somehow .__init__
was forgotten in the previous tentatives)
But
 it does not fix the problem in #23875
 it may behave uncorrectly for morphisms from number fields to rings that are not fields
comment:13 Changed 5 years ago by
Commit:  49ddc5d90892a2e7343539703f8aa2ce827aa16d → 3676535ddcf1310b2f3c75b884eaacf94036bd37 

Branch pushed to git repo; I updated commit sha1. New commits:
3676535  trac 23647 better category handling in Number fields morphisms

comment:14 Changed 5 years ago by
Here is another tentative, let us see what patchbot says.
New commits:
3676535  trac 23647 better category handling in Number fields morphisms

comment:15 Changed 5 years ago by
Status:  new → needs_review 

comment:16 Changed 5 years ago by
Commit:  3676535ddcf1310b2f3c75b884eaacf94036bd37 → 768891be49e9a4b2409bc9b1bc731e306ac4e67d 

Branch pushed to git repo; I updated commit sha1. New commits:
768891b  trac 23647 add doctest

comment:17 Changed 5 years ago by
Green patchbot. Travis, do you think that this is good enough ?
We could also try to find the best category given the target category..
comment:18 Changed 5 years ago by
Authors:  → Frédéric Chapoton 

Reviewers:  → Travis Scrimshaw 
Status:  needs_review → positive_review 
I think it improves things and is fairly close to a proper (IMO) fix of determining the category directly from the input. We can worry about doing more if this becomes an issue again somewhere else.
comment:19 Changed 5 years ago by
Branch:  public/23647v2 → 768891be49e9a4b2409bc9b1bc731e306ac4e67d 

Resolution:  → fixed 
Status:  positive_review → closed 
Travis, can you help, please ? This prevents from using richcmp for number fields morphisms.