put number fields in Fields().Infinite()

[fchapoton]

and the same for QQ of course.

This fixes a bug with Hom where the meet computed is a join category, in which case X is not a subclass of category.parent_class. This causes problems with the number fields meeting with matrix spaces.

Follow up at #24432.

CC: @nthiery @tscrim

Component: number fields

Author: Frédéric Chapoton, Vincent Delecroix

Branch/Commit: 2008ba1

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/23418

[fchapoton]

New commits:

2f98551

trac 23418 fixing some doctests in rings and category folders

[fchapoton]

Commit: 2f98551

[fchapoton]

Branch: u/chapoton/23418

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

a06ebbc

trac 23418 correct more doctests

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 2f98551 to a06ebbc

[fchapoton]

comment:3

This triggers some problem here:

sage -t --long src/sage/structure/parent.pyx  # 6 doctests failed

[fchapoton]

comment:4

It seems that the morphisms from Sets take over the morphisms from Rings..

[tscrim]

comment:5

Failures are related to this:

sage: x = QQ['x'].0
sage: t = abs(ZZ.random_element(10^6))
sage: K = NumberField(x^2 + 2*3*7*11, "a"+str(t))
sage: a = K.gen()

sage: Hom(K, a.matrix().parent())
Set of Morphisms from Number Field in a791947 with defining polynomial x^2 + 462 to Full MatrixSpace of 2 by 2 dense matrices over Rational Field in Category of infinite rings
<class 'sage.categories.homset.Homset_with_category'>

compared to vanilla Sage:

sage: Hom(K, a.matrix().parent())
Set of Homomorphisms from Number Field in a395598 with defining polynomial x^2 + 462 to Full MatrixSpace of 2 by 2 dense matrices over Rational Field
sage: type(Hom(K, a.matrix().parent()))
<class 'sage.rings.homset.RingHomset_generic_with_category'>

This differences comes from the fact that the category becomes a join category (in the branch) because that is the meet category computed by Hom:

sage: K.category()._meet_(a.matrix().parent().category())
Category of infinite rings
sage: type(_)
<class 'sage.categories.category.JoinCategory_with_category'>

versus vanilla:

sage: K.category()._meet_(a.matrix().parent().category())
Category of rings
sage: type(_)
<class 'sage.categories.rings.Rings_with_category'>

Now, K is not a subclass of the meet category's parent_class, which causes the Cat.parent_class._Hom_ method to now raise an error:

sage: Cat = K.category()._meet_(a.matrix().parent().category())
sage: Cat.parent_class
<class 'sage.categories.category.JoinCategory.parent_class'>
sage: Cat.parent_class._Hom_(K, a.matrix().parent(), Cat)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-aa7f8d9cdf73> in <module>()
----> 1 Cat.parent_class._Hom_(K, a.matrix().parent(), Cat)
TypeError: unbound method _Hom_() must be called with JoinCategory.parent_class instance as first argument (got NumberField_quadratic_with_category instance instead)

versus vanilla

sage: Cat = K.category()._meet_(a.matrix().parent().category())
sage: Cat.parent_class
<class 'sage.categories.rings.Rings.parent_class'>
sage: Cat.parent_class._Hom_(K, a.matrix().parent(), Cat)
Set of Homomorphisms from Number Field in a395598 with defining polynomial x^2 + 462 to Full MatrixSpace of 2 by 2 dense matrices over Rational Field

[fchapoton]

Changed branch from u/chapoton/23418 to u/chapoton/23418_bare

[fchapoton]

New commits:

ba2ec5b

Merge commit '2f98551bf0aa8fc99f98cf2cdecf2fdaf584e5df' in 8.1.b5

[fchapoton]

Changed commit from a06ebbc to ba2ec5b

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from ba2ec5b to 4e88b49

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

4e88b49

fix

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

6d3e532

trac 23418 fix

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 4e88b49 to 6d3e532

[videlec]

Changed author from Frédéric Chapoton to Frédéric Chapoton, Vincent Delecroix

[videlec]

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
 and the same for QQ of course
+
+follow up at #24432

[videlec]

Changed commit from 6d3e532 to d2bf048

[videlec]

New commits:

d2bf048

23418: number fields are infinite

[videlec]

Changed branch from u/chapoton/23418_bare to u/vdelecroix/23418

[videlec]

comment:10

the problem from comment:5 is solved in d2bf048 via

--- a/src/sage/categories/homset.py
+++ b/src/sage/categories/homset.py
@@ -402,7 +402,7 @@ def Hom(X, Y, category=None, check=True):
                 #   will be called twice.
                 # - This is bound to fail if X is an extension type and
                 #   does not actually inherit from category.parent_class
-                H = category.parent_class._Hom_(X, Y, category = category)
+                H = X.category().parent_class._Hom_(X, Y, category = category)
             except (AttributeError, TypeError):
                 # By default, construct a plain homset.
                 H = Homset(X, Y, category = category, check=check)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a632bf6

23418: number fields are infinite

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from d2bf048 to a632bf6

[fchapoton]

comment:12

Looks good to me. Maybe Travis can confirm ?

[tscrim]

comment:13

No, I don't think this is correct because now it ignores the category input, and there might be a situation where this works with category being passed in (say, Modules() instead of Algebras()). So this change is a regression in my mind because the Homset classes do not have the inheritance of the categories. (In fact, in some ways, the error is because of this desired behavior.) Perhaps what we can do is do something special for join categories and testing its super_categories?

[videlec]

comment:14

Replying to @tscrim:

No, I don't think this is correct because now it ignores the category input

Strictly speaking it does not, since the category argument is passed in the call

H = X.category().parent_class._Hom_(X, Y, category = category)

and there might be a situation where this works with category being passed in (say, Modules() instead of Algebras()).

Do you have any example of this? I might understand the objection, but my "cheap fix" is indeed fixing problems and not creating new ones (as far as doctesting is complete).

So this change is a regression in my mind because the Homset classes do not have the inheritance of the categories. (In fact, in some ways, the error is because of this desired behavior.) Perhaps what we can do is do something special for join categories and testing its super_categories?

As you might have noticed, the call line 405 in categories/homset.py is already hackish. Parents implementing their own _Hom_ method should by themselves call a super (or equivalent). Before doing my "cheap fix", I thought that the good solution would be to remove this line 405 and fix the parents. Do you agree that this would be the solution?

If you agree with the above, there are two concrete ways of doing this:

• let my cheap fix pass and remove line 405 afterwards
• remove line 405 in a dependency ticket

[tscrim]

comment:15

Replying to @videlec:

Replying to @tscrim:

No, I don't think this is correct because now it ignores the category input

Strictly speaking it does not, since the category argument is passed in the call

H = X.category().parent_class._Hom_(X, Y, category = category)

No, _Hom_ ignores the category input.

and there might be a situation where this works with category being passed in (say, Modules() instead of Algebras()).

Do you have any example of this? I might understand the objection, but my "cheap fix" is indeed fixing problems and not creating new ones (as far as doctesting is complete).

No, your hack is creating new problems. For example, suppose X is an algebra with:

• basis {a, b},
• multiplication a^2 = b^2 = ab = a.

If we want to construct a map f: X -> X given by a,b |-> b. This is not an algebra morphism, but instead only a module morphism. If we have Algebras._Hom_, which expects the category to be algebras (as it should), then this will break horribly and there is no way to use a Modules._Hom_ (assuming it is also implemented).

For an example where this is currently implemented, we can replace Algebras with HighestWeightCrystals and Modules with Crystals. I have also constructed similar morphisms as above for my research.

So this change is a regression in my mind because the Homset classes do not have the inheritance of the categories. (In fact, in some ways, the error is because of this desired behavior.) Perhaps what we can do is do something special for join categories and testing its super_categories?

As you might have noticed, the call line 405 in categories/homset.py is already hackish. Parents implementing their own _Hom_ method should by themselves call a super (or equivalent).

I am not sure this would be the same behavior. There are two different things going on:

1. If a parent does not implement a _Hom_ method, then it should fall back to the specified category. Continuing my above example, this would only default to Algebras._Hom_, which would be wrong.
2. The other behavior is if the homset returned by X._Hom_ does not support the more general category, say, because it takes advantage of the extra structure (say, is only uses the algebra generating set and not the [possibly infinite] basis). One should expect that X.category()._Hom_ would also take advantage of that extra structure as well and would similarly forbid more general categories.

Before doing my "cheap fix", I thought that the good solution would be to remove this line 405 and fix the parents. Do you agree that this would be the solution?

It doesn't make sense to me for the parent/category implementation of _Hom_ to do super calls when something fails. I think it would create a lot of redundant logic (the error handling). Also, everything in between X.category() and the specified category would likely (needlessly) fail, making these calls significantly slower when there is a large category difference. While this is a solution, I think it is a very bad solution.

I think special handling the join categories is going to be the proper fix as join categories are somewhat special in the category framework. While it may be just fixing the symptoms, it does not have the drawbacks that the other approaches have. I will push something today.

[videlec]

comment:16

Replying to @tscrim:

Replying to @videlec:

Replying to @tscrim:

No, I don't think this is correct because now it ignores the category input

Strictly speaking it does not, since the category argument is passed in the call

H = X.category().parent_class._Hom_(X, Y, category = category)

No, _Hom_ ignores the category input.

and there might be a situation where this works with category being passed in (say, Modules() instead of Algebras()).

Do you have any example of this? I might understand the objection, but my "cheap fix" is indeed fixing problems and not creating new ones (as far as doctesting is complete).

No, your hack is creating new problems. [SNIP]

This would better be doctested! Could you come up with concrete examples to be added to the branch? Especially because we are going to play with the _Hom_ code.

So this change is a regression [...]

I will push something today.

Thanks! You might want to use another ticket or update the description of this one.

[tscrim]

comment:17

Replying to @videlec:

This would better be doctested! Could you come up with concrete examples to be added to the branch? Especially because we are going to play with the _Hom_ code.

I believe I have a concrete example and will add it.

So this change is a regression [...]

I will push something today.

Thanks! You might want to use another ticket or update the description of this one.

No problem. Thanks for your work on this.

[tscrim]

Changed branch from u/vdelecroix/23418 to public/number_fields/in_infinite_fields-23418

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
-and the same for QQ of course
+and the same for QQ of course.

-follow up at #24432
+This fixes a bug with `Hom` where the meet computed is a join category, in which case `X` is not a subclass of `category.parent_class`. This causes problems with the number fields meeting with matrix spaces.
+
+Follow up at #24432.

[tscrim]

comment:18

Actually, from looking at it and thinking about it more, the category of highest weight crystals is a full subcategory, so it does not quite work (in contrast to algebras and modules). So instead I just manufactured a minimal example that fails with your change.

Here is a branch that does special handling of JoinCategory. It passes all the tests that were previously failing.

---

New commits:

39b0560	Merge branch 'u/vdelecroix/23418' of git://trac.sagemath.org/sage into public/number_fields/in_infinite_fields-23418
cdbc67d	Reverting change and adding test showing the behavior.
b7f3fa6	Special handling of the join categories for Hom.

[tscrim]

Changed commit from a632bf6 to b7f3fa6

[videlec]

comment:19

The following loop is randomly choosing the last working call.

for C in cats:
    try:
        H = C.parent_class._Hom_(X, Y, category=category)
    except (AttributeError, TypeError):
        pass

If you want to keep this hack, wouldn't it be better this way

for C in cats:
    try:
        H = C.parent_class._Hom_(X, Y, category=category)
    except (AttributeError, TypeError):
        pass
    else:
        break

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

bf01aa1

Break out once H is found.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b7f3fa6 to bf01aa1

[tscrim]

comment:21

Good point; fixed.

[tscrim]

comment:22

Green bot.

[videlec]

comment:23

Sorry to have forgotten!

[vbraun]

comment:24

git reset --hard Hsage -t --long --warn-long 69.0 src/sage/structure/parent.pyx
**********************************************************************
File "src/sage/structure/parent.pyx", line 1855, in sage.structure.parent.Parent.hom._generic_coerce_map
Failed example:
    QQ['x', 'y']._generic_coerce_map(QQ).category_for()
Expected:
    Category of unique factorization domains
Got:
    Category of infinite unique factorization domains
EAD~
**********************************************************************
1 item had failures:
   1 of   4 in sage.structure.parent.Parent.hom._generic_coerce_map
    [407 tests, 1 failure, 3.07 s]

[tscrim]

comment:25

I cannot reproduce this locally, so we might have to wait for the next beta. The doctest failure output should be the correct output. Yet, the test after it should probably also be Category of infinite euclidean domains, so something subtle might be going on too...

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from bf01aa1 to 2008ba1

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

2714d24	Merge branch 'public/number_fields/in_infinite_fields-23418' in 8.2.b3
2008ba1	fix doctest

[fchapoton]

comment:27

I fixed the doctest

[tscrim]

comment:28

So the failure was because of #24413, whereas the second test would need a similar treatment for power series rings. This is good enough for now.

[vbraun]

Changed branch from public/number_fields/in_infinite_fields-23418 to 2008ba1