valuation error
Closed this issue · 28 comments
Sage makes a calculation error with valuation.
After defining
E = EllipticCurve('15a1')
D = -11
p = 7
P = E.heegner_point(D)
Q = P.point_exact()
R = PolynomialRing(QQ, 'x')
x = R.gen()
K.<a> = NumberField (x^2 - D)
E_K = E.base_extend(K)
I = K.ideal(p)
v = I.prime_factors()[0]
E_loc = E_K.reduction(v)
Np = E_loc.cardinality()
Q2 = Np * Q
Q2.clear_denominators()
a = Q2.xy()[0]
b = Q2.xy()[1]
c = a / b
we get:
sage: [x.valuation(v) for x in (a, b, c)]
[-1, -2, 0]
Clearly an error as c.valuation(v)
should be 1, not 0.
Furthermore the v-valuations of b and c
should add up to that of a.
CC: @orlitzky @mmasdeu @categorie @roed314
Component: number theory
Keywords: valuation
Author: Frédéric Chapoton
Branch/Commit: d8f5f13
Reviewer: Edgar Costa
Issue created by migration from https://trac.sagemath.org/ticket/29215
There is a bug in a valuation, but the bug is that we didn't raise an error when
we called with an ideal over a different number field.
Perhaps we want to add a check that self.number_field() == P.number_field()
sage: Q.curve().base_ring()
Number Field in a with defining polynomial x^2 - 3*x + 18
sage: a.parent()
Number Field in a with defining polynomial x^2 - 3*x + 18
sage: b.parent()
Number Field in a with defining polynomial x^2 - 3*x + 18
sage: v.parent()
Monoid of ideals of Number Field in a with defining polynomial x^2 + 11
Furthemore,
sage: v7 = a.parent().ideal(7).factor()[0][0]
sage: print(a.valuation(v7), b.valuation(v7), c.valuation(v7))
-2 -3 1
Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.
Moving to 9.4, as 9.3 has been released.
Branch: u/chapoton/29215
Author: Frédéric Chapoton
hmm, the new check triggers a doctest failure in
src/sage/modular/pollack_stevens/modsym.py
Experts, please help
The issue seems to be that hecke_eigenvalue_field and the base ring of phi are different but isomorphic.
One easy to fix the test is to have K = phi.base_ring() and if Marc Masdeu agrees, create another issue regarding the disparity between the presentation of f.hecke_eigenvalue_field() and phi.base_ring().
sage: f = Newforms(32, 8, names='a')[1]
....: K = f.hecke_eigenvalue_field()
....: a = f[3]
....: from sage.modular.pollack_stevens.space import ps_modsym_from_simple_modsym_space
....: phi = ps_modsym_from_simple_modsym_space(f.modular_symbols(1))
sage: p = K.ideal(3, 1/16*a + 3/2)
sage: q = p.smallest_integer()
sage: aq = phi.Tq_eigenvalue(q)
sage: phi.base_ring()
Number Field in alpha with defining polynomial x^2 + 32*x - 9984
sage: aq.parent()
Number Field in alpha with defining polynomial x^2 + 32*x - 9984
sage: p.parent()
Monoid of ideals of Number Field in a1 with defining polynomial x^2 + 16*x - 2496
sage: K
Number Field in a1 with defining polynomial x^2 + 16*x - 2496
sage: K.is_isomorphic(aq.parent())
True
It's not a problem with the Pollack-Stevens modular symbols:
sage: f = Newforms(32, 8, names='a')[1]
sage: K = f.hecke_eigenvalue_field()
sage: L = f.modular_symbols(1).eigenvalue(1).parent()
sage: K == L
False
sage: K.is_isomorphic(L)
True
Yeah, I also think it is okay for them to pick different representations of the base ring.
Thus, my fix would be to replace the current test
sage: f = Newforms(32, 8, names='a')[1]
sage: K = f.hecke_eigenvalue_field()
sage: a = f[3]
sage: from sage.modular.pollack_stevens.space import ps_modsym_from_simple_modsym_space
sage: phi = ps_modsym_from_simple_modsym_space(f.modular_symbols(1))
sage: phi.is_ordinary(K.ideal(3, 1/16*a + 3/2)) != phi.is_ordinary(K.ideal(3, 1/16*a + 5/2))
True
sage: phi.is_ordinary(3)
Traceback (most recent call last):
...
TypeError: P must be an ideal
with
sage: f = Newforms(32, 8, names='a')[1]
sage: from sage.modular.pollack_stevens.space import ps_modsym_from_simple_modsym_space
sage: phi = ps_modsym_from_simple_modsym_space(f.modular_symbols(1))
sage: K = phi.base_ring()
sage: a = K.gen()
sage: phi.is_ordinary(K.ideal(3, 1/32*a + 1/2)) != phi.is_ordinary(K.ideal(3, 1/32*a + 3/2))
True
sage: phi.is_ordinary(3)
Traceback (most recent call last):
...
TypeError: P must be an ideal
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a260f32 | more tests for .valuation |
new branch, with modified doctest as suggested
Branch pushed to git repo; I updated commit sha1. New commits:
14449e1 | fix doctest |
Reviewer: Edgar Costa
Thank you very much! It looks good to me.
Feel free to change the status to a positive review once the tests pass.
green bot, so setting to positive
Description changed:
---
+++
@@ -1,24 +1,35 @@
-Sage makes a calculation error with valuation. See this code
+Sage makes a calculation error with valuation.
+
+After defining
```
-E=EllipticCurve('15a1')
-D=-11
-p=7
-P=E.heegner_point(D)
-Q=P.point_exact()
-R=PolynomialRing(QQ,'x')
-x=R.gen()
-K.<a>=NumberField (x^2-D)
-E_K=E.base_extend(K)
-I=K.ideal(p)
-v=I.prime_factors()[0]
-E_loc=E_K.reduction(v)
-Np=E_loc.cardinality()
-Q2=Np*Q
+E = EllipticCurve('15a1')
+D = -11
+p = 7
+P = E.heegner_point(D)
+Q = P.point_exact()
+R = PolynomialRing(QQ, 'x')
+x = R.gen()
+K.<a> = NumberField (x^2 - D)
+E_K = E.base_extend(K)
+I = K.ideal(p)
+v = I.prime_factors()[0]
+E_loc = E_K.reduction(v)
+Np = E_loc.cardinality()
+Q2 = Np * Q
Q2.clear_denominators()
-a=Q2.xy()[0]
-b=Q2.xy()[1]
-c=a/b
-print(a.valuation(v), b.valuation(v), c.valuation(v))
+a = Q2.xy()[0]
+b = Q2.xy()[1]
+c = a / b
```
-This returns -1,-2,0. There is clearly an error as c.valuation(v) should be 1.
+we get:
+
+```
+sage: [x.valuation(v) for x in (a, b, c)]
+[-1, -2, 0]
+```
+Clearly an error as `c.valuation(v)`
+should be `1`, not `0`.
+
+Furthermore the `v`-valuations of `b` and `c`
+should add up to that of `a`.Volker is now trying to merge this ticket but I am getting this error with it
sage -t --long --random-seed=338744033241951726879691723867373127890 /usr/lib/python3.10/site-packages/sage/modular/pollack_stevens/modsym.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/modular/pollack_stevens/modsym.py", line 723, in sage.modular.pollack_stevens.modsym.PSModularSymbolElement.is_ordinary
Failed example:
phi.is_ordinary(K.ideal(3, 1/32*a + 1/2)) != phi.is_ordinary(K.ideal(3, 1/32*a + 3/2))
Exception raised:
Traceback (most recent call last):
File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/usr/lib/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.modular.pollack_stevens.modsym.PSModularSymbolElement.is_ordinary[13]>", line 1, in <module>
phi.is_ordinary(K.ideal(Integer(3), Integer(1)/Integer(32)*a + Integer(1)/Integer(2))) != phi.is_ordinary(K.ideal(Integer(3), Integer(1)/Integer(32)*a + Integer(3)/Integer(2)))
File "/usr/lib/python3.10/site-packages/sage/modular/pollack_stevens/modsym.py", line 744, in is_ordinary
return aq.valuation(p) == 0
File "sage/rings/number_field/number_field_element.pyx", line 3850, in sage.rings.number_field.number_field_element.NumberFieldElement.valuation (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_10/cythonized/sage/rings/number_field/number_field_element.cpp:32228)
raise ValueError("P must be prime")
ValueError: P must be prime
**********************************************************************
Sorry about my typo.
The ideals should be (3, 1/32*alpha + 5/2) and (3, 1/32*alpha + 3/2).
However, a better way to do this is:
sage: phi = ps_modsym_from_simple_modsym_space(f.modular_symbols(1))
sage: (p1, _), (p2, _) = phi.base_ring().ideal(3).factor()
sage: phi.is_ordinary(p1) != phi.is_ordinary(p2)
True
this avoids the awkward construction of the ideals via generators and makes clear why we choose these two ideals.
thanks, modified
Once again, the changes look good to me. Feel free to change the status to a positive review once the tests pass.
However, I'm still confused about how the tests passed with the previous typo...
morally green bot, setting to positive
Changed branch from u/chapoton/29215 to d8f5f13