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: sage-8.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:

Status badges

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 Frédéric Chapoton

Cc: Travis Scrimshaw added

Travis, can you help, please ? This prevents from using richcmp for number fields morphisms.

comment:2 Changed 5 years ago by Travis Scrimshaw

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:3 Changed 5 years ago by Frédéric Chapoton

Do you have branch for what you tried ?

comment:4 Changed 5 years ago by Travis Scrimshaw

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 Frédéric Chapoton

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.

Last edited 5 years ago by Frédéric Chapoton (previous) (diff)

comment:6 Changed 5 years ago by Frédéric Chapoton

Branch: public/23647
Commit: ec7769501def864ad49a7282136cbc64e86be010

Here is my branch.


New commits:

ec77695tentative

comment:7 Changed 5 years ago by Frédéric Chapoton

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 Travis Scrimshaw

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 Travis Scrimshaw

I believe UniversalCyclotomicField() is not a number field as I believe is an infinite-dimensional field extension of Q.

comment:10 Changed 5 years ago by Frédéric Chapoton

Branch: public/23647public/23647-v2
Commit: ec7769501def864ad49a7282136cbc64e86be01056c31afa0082e075c5644c041ba4a30827e1434b

let us see what the patchbots say


New commits:

56c31aftrac 23647

comment:11 Changed 5 years ago by git

Commit: 56c31afa0082e075c5644c041ba4a30827e1434b49ddc5d90892a2e7343539703f8aa2ce827aa16d

Branch pushed to git repo; I updated commit sha1. New commits:

49ddc5dtrac 23647 fix doctest

comment:12 Changed 5 years ago by Frédéric Chapoton

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 git

Commit: 49ddc5d90892a2e7343539703f8aa2ce827aa16d3676535ddcf1310b2f3c75b884eaacf94036bd37

Branch pushed to git repo; I updated commit sha1. New commits:

3676535trac 23647 better category handling in Number fields morphisms

comment:14 Changed 5 years ago by Frédéric Chapoton

Here is another tentative, let us see what patchbot says.


New commits:

3676535trac 23647 better category handling in Number fields morphisms

comment:15 Changed 5 years ago by Frédéric Chapoton

Status: newneeds_review

comment:16 Changed 5 years ago by git

Commit: 3676535ddcf1310b2f3c75b884eaacf94036bd37768891be49e9a4b2409bc9b1bc731e306ac4e67d

Branch pushed to git repo; I updated commit sha1. New commits:

768891btrac 23647 add doctest

comment:17 Changed 5 years ago by Frédéric Chapoton

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 Travis Scrimshaw

Authors: Frédéric Chapoton
Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_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 Volker Braun

Branch: public/23647-v2768891be49e9a4b2409bc9b1bc731e306ac4e67d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.