Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#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:

Status badges

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)

trac_5499.patch (4.6 KB) - added by roed 12 years ago.
trac_5499_2.patch (6.7 KB) - added by roed 12 years ago.
Fixes laurents series, as well as p-adic l-series code affected by change in direction of coercion
trac_5499_2.2.patch (7.0 KB) - added by mabshoff 12 years ago.
This is the patch I imported which gives credit to David

Download all attachments as: .zip

Change History (14)

Changed 12 years ago by roed

comment:1 Changed 12 years ago by roed

  • 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 robertwb

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) not O((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?

comment:3 Changed 12 years ago by mabshoff

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 mabshoff

  • 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 kedlaya

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 kedlaya

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 roed

  • 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 roed

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 mabshoff

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 was

  • 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 mabshoff

  • 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 robertwb

Regarding "change in direction of the coercion maps", this should at least be accompanied by some doctests to illistrate what's going on...

Changed 12 years ago by mabshoff

This is the patch I imported which gives credit to David

Note: See TracTickets for help on using tickets.