#5499 closed defect (fixed)
[with patch; positive review] Wrong precision when creating p-adic field element
Reported by: | kedlaya | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-3.4.1 |
Component: | number theory | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This was originally reported on ticket #5076 but seems to be a separate issue.
sage: K = Qp(11,8) sage: a = 11^-2 + O(11^5) sage: a 11^-2 + O(11^3)
By contrast:
sage: K = Qp(11,8) sage: 11^(-2) + K(O(11^5)) 11^-2 + O(11^5)
Note that
sage: O(11^5).parent() 11-adic Ring with capped relative precision 5 sage: O(11^5).parent() == K False
Attachments (3)
Change History (14)
Changed 12 years ago by
comment:1 Changed 12 years ago by
- Summary changed from Wrong precision when creating p-adic field element to [with patch; needs review] Wrong precision when creating p-adic field element
comment:2 Changed 12 years ago by
comment:3 Changed 12 years ago by
David: Any chance you could answer Robert's question since this patch is holding up the merge of #4637?
Cheers,
Michael
comment:4 Changed 12 years ago by
- Summary changed from [with patch; needs review] Wrong precision when creating p-adic field element to [with patch; needs work] Wrong precision when creating p-adic field element
This patch causes many doctest failures:
sage -t -long devel/sage/sage/schemes/elliptic_curves/padic_lseries.py # 1 doctests failed sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctests failed sage -t -long devel/sage/sage/schemes/elliptic_curves/ell_tate_curve.py # 1 doctests failed sage -t -long devel/sage/doc/fr/tutorial/tour_polynomial.rst # 1 doctests failed sage -t -long devel/sage/sage/schemes/elliptic_curves/formal_group.py # 8 doctests failed sage -t -long devel/sage/doc/en/tutorial/tour_polynomial.rst # 1 doctests failed sage -t -long devel/sage/sage/rings/laurent_series_ring_element.pyx # 48 doctests failed sage -t -long devel/sage/sage/rings/laurent_series_ring.py # 3 doctests failed sage -t -long devel/sage/sage/rings/big_oh.py # 2 doctests failed sage -t -long devel/sage/sage/schemes/elliptic_curves/padics.py # Segfault
The last seems to get very slow, i.e. only the last step in the following computation
Trying: E = EllipticCurve('37a')###line 585:_sage_ >>> E = EllipticCurve('37a') Expecting nothing ok Trying: E.is_supersingular(Integer(3))###line 586:_sage_ >>> E.is_supersingular(3) Expecting: True ok Trying: h = E.padic_height(Integer(3), Integer(5))###line 588:_sage_ >>> h = E.padic_height(3, 5) Expecting nothing ok Trying: h(E.gens()[Integer(0)])###line 589:_sage_ >>> h(E.gens()[0]) Expecting: (2*3 + 2*3^2 + 3^3 + 2*3^4 + 2*3^5 + O(3^6), 3^2 + 3^3 + 3^4 + 3^5 + O(3^7))
takes more than 3 minutes CPU time on sage.math.
Cheers,
Michael
comment:5 Changed 12 years ago by
I was originally unsure whether this was simply because those doctests depended on the old behavior. But in fact there is a real problem with the patch, as I extracted from the second file on Michael's list.
sage: E=EllipticCurve('389a1') sage: X,Y=E.modular_parametrization() sage: q = X.parent().gen() sage: E.defining_polynomial()(X,Y,1) 869*q^11 + 2151*q^12 - 2768*q^13 + O(q^14) sage: E.defining_polynomial()(X,Y,1) + O(q^11) 870*q^11 + 2151*q^12 - 2768*q^13 + O(q^14)
comment:6 Changed 12 years ago by
To clarify my previous comment: before the patch, we have correctly:
sage: R.<q> = LaurentSeriesRing(Rationals()) sage: O(q^14) O(q^14)
but afterwards we have:
sage: R.<q> = LaurentSeriesRing(Rationals()) sage: O(q^14) q^14
comment:7 Changed 12 years ago by
- Summary changed from [with patch; needs work] Wrong precision when creating p-adic field element to [with patch; needs review] Wrong precision when creating p-adic field element
I fixed the problem with the laurent series ring, which was also causing the doctest failures.
The change in direction of the coercion maps is one that I discussed with Kiran at AWS. I think it's a good idea, though it is a somewhat different issue.
Changed 12 years ago by
Fixes laurents series, as well as p-adic l-series code affected by change in direction of coercion
comment:8 Changed 12 years ago by
Cool, trac_5499_2.patch only passes doctests for my 3.4.1.rc3 merge tree.
Cheers,
Michael
comment:9 Changed 12 years ago by
- Summary changed from [with patch; needs review] Wrong precision when creating p-adic field element to [with patch; positive review] Wrong precision when creating p-adic field element
comment:10 Changed 12 years ago by
- Milestone changed from sage-3.4.2 to sage-3.4.1
- Resolution set to fixed
- Status changed from new to closed
Merged trac_5499_2.patch in Sage 3.4.1.rc3.
Cheers,
Michael
comment:11 Changed 12 years ago by
Regarding "change in direction of the coercion maps", this should at least be accompanied by some doctests to illistrate what's going on...
The explanation looks good, I'm not sure how one would get around this without doing something lazily. Also, +1 to only allowing
O(x^n)
notO((x+1)^n)
However, what's up with the change of comparison direction in the last three files of the patch? Is this fixing an separate bug?