sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 487 forks source link

Fix comparison of automorphisms of Eisenstein extensions #30938

Open pjbruin opened 4 years ago

pjbruin commented 4 years ago

In SageMath 9.3.beta1:

sage: R.<x> = QQ[]
....: K = Qp(19)
....: L.<b> = K.extension(x^2 - 19)
....: a = L.hom([b], codomain=L)
....: id = L.Hom(L).identity()
....: a == id
Traceback (most recent call last):
...
TypeError: 'NoneType' object is not iterable
During handling of the above exception, another exception occurred:
...
AttributeError: 'NoneType' object has no attribute 'list'
During handling of the above exception, another exception occurred:
...
TypeError: cannot convert x to a p-adic element
During handling of the above exception, another exception occurred:
...
TypeError: None fails to convert into the map's domain 19-adic Eisenstein Extension Field in b defined by x^2 - 19, but a `pushforward` method is not properly implemented

CC: @jdemeyer @tscrim @roed314

Component: padics

Author: Peter Bruin

Branch/Commit: u/pbruin/30938-padic_element_from_None @ 64063cf

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

pjbruin commented 4 years ago

Branch: u/pbruin/30938-padic_element_from_None

pjbruin commented 4 years ago
comment:1

The attached branch fixes this by implementing conversion of None to an element of L, which already works for most rings. There are other solutions, but those seem more complicated.

pjbruin commented 4 years ago

Commit: 64063cf

pjbruin commented 4 years ago

Author: Peter Bruin

mwageringel commented 3 years ago
comment:2

It seems comparison of ring morphisms with the identity morphism might be a wider problem. See #29632 for a similar issue.

mwageringel commented 3 years ago
comment:3

The None in the error comes from the generic implementation of _richcmp_ in sage.categories.morphism which performs a computation like this

sage: L.gen(0)._lmul_(K.gen(0)) is None
True

which looks like _lmul_ is not implemented. It seems that the implementation of the _richcmp_ method assumes that the result is

sage: L.gen(0) * K.gen(0)
b^3 + O(b^43)

so treating None as zero here will not correctly resolve this problem.

Another issue with the implementation of _richcmp_ is that the <ModuleElement>e, on which _lmul_ is called, might be zero if the last generator is 0. For example in this case:

sage: K.<a> = QuadraticField(-1)
sage: R.<x,y> = K['x,y'].quotient('y')
sage: id = R.Hom(R).identity()
sage: f = R.hom(R.gens(), R, base_map=K.hom([-a], K))
sage: f == id  # should be False
True

As e=y is zero, also y * a is zero, so that comparing the images f(y * a) and id(y * a) is not meaningful. (Currently, this example fails because _lmul_ returns None, though, and therefore also fails for the quotient by x.)

So at least the implementation should pick a module element e that is non-zero, but I am not sure if that is already a sufficient condition for checking equality.

pjbruin commented 3 years ago
comment:4

This _richcmp_() method was totally rewritten in #24281; adding the author and reviewer of that ticket in CC.

pjbruin commented 3 years ago
comment:5

Replying to @mwageringel:

It seems comparison of ring morphisms with the identity morphism might be a wider problem. See #29632 for a similar issue.

Possibly also related: #28617

kwankyu commented 3 years ago
comment:6

Replying to @mwageringel:

The None in the error comes from the generic implementation of _richcmp_ in sage.categories.morphism which performs a computation like this

sage: L.gen(0)._lmul_(K.gen(0)) is None
True

which looks like _lmul_ is not implemented. It seems that the implementation of the _richcmp_ method assumes that the result is

sage: L.gen(0) * K.gen(0)
b^3 + O(b^43)

so treating None as zero here will not correctly resolve this problem.

Right. Implementing scalar multiplication via _lmul_ for the ring elements will solve the problem.

kwankyu commented 3 years ago
comment:7

Now the patch for #28617 also fixes the problem of this ticket.

pjbruin commented 3 years ago
comment:8

Replying to @kwankyu:

Now the patch for #28617 also fixes the problem of this ticket.

With the patch from #28617 it no longer raises an error, but the answer is wrong:

sage: R.<x> = QQ[]
....: K = Qp(19)
....: L.<b> = K.extension(x^2 - 19)
....: a = L.hom([b], codomain=L)
....: id = L.Hom(L).identity()
....: a == id
False

The result should be True.

kwankyu commented 3 years ago
comment:9

Does a behave correctly?

sage: a(K.gen(0)) == K.gen(0)
False
sage: a(K.gen(0)) == L(K.gen(0))
False
sage: K.gen()
19 + O(19^21)
sage: L(K.gen(0))
b^2 + O(b^42)
sage: a(K.gen(0))
b^2 + 18*b^40 + O(b^42)
pjbruin commented 3 years ago
comment:10

This is funny. The output of L(K.gen(0)) is correct (since 19 = b^2 in L), but the output of a(K.gen(0)) has a wrong term 18*b^40... Same for a(b^2) (but not for a(b)):

sage: b
b + O(b^41)
sage: a(b)
b + O(b^41)
sage: b^2
b^2 + O(b^42)
sage: a(b^2)
b^2 + 18*b^40 + O(b^42)
pjbruin commented 3 years ago
comment:11

This looks suspicious (the term 18*19^20 should probably not be there):

sage: c = b^2
sage: c._polynomial_list()
[19 + 18*19^20 + O(19^21)]

And this:

sage: f, k = c._ntl_rep_abs(); f
[676619522235827247480400837]
sage: K(ZZ(f[0]))
19 + 18*19^20 + O(19^21)
pjbruin commented 3 years ago
comment:12

Relevant input for the above example:

sage: K = Qp(19)
sage: R.<x> = K[]
sage: L.<b> = K.extension(x^2 - 19)
sage: c = b^2
sage: f, k = c._ntl_rep_abs(); f
[676619522235827247480400837]
sage: K(ZZ(f[0]))
19 + 18*19^20 + O(19^21)

Maybe David Roe (CC) can say something about this?

mkoeppe commented 3 years ago
comment:13

Moving to 9.4, as 9.3 has been released.

mkoeppe commented 3 years ago
comment:14

Setting a new milestone for this ticket based on a cursory review.

mkoeppe commented 2 years ago
comment:15

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.