Opened 14 years ago

Closed 14 years ago

#4406 closed defect (fixed)

[with patch, positive review] make polynomial truncation cpdef method

Reported by: robertwb Owned by: tbd
Priority: major Milestone: sage-3.2
Component: algebra Keywords:
Cc: craigcitro Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Currently we have _c variants, some of which call one direction, and some which call the other.

Attachments (2)

4406-cpdef-truncate.patch (5.4 KB) - added by robertwb 14 years ago.
4406-truncate-fix.patch (737 bytes) - added by robertwb 14 years ago.

Download all attachments as: .zip

Change History (13)

Changed 14 years ago by robertwb

comment:1 Changed 14 years ago by robertwb

  • Cc craigcitro added
  • Summary changed from make polynomail truncation cpdef method to [with patch, needs review] make polynomail truncation cpdef method

This wasn't as invasive as I had expected. Apply on top of fix at #2462.

comment:2 Changed 14 years ago by mabshoff

  • Milestone set to sage-3.2
  • Summary changed from [with patch, needs review] make polynomail truncation cpdef method to [with patch, needs review] make polynomial truncation cpdef method

Patch looks good to me, but I will wait on a review #2462 before testing this. Also fixed a spelling mistake in the subject.

Cheers,

Michael

comment:3 Changed 14 years ago by mabshoff

  • Milestone changed from sage-3.2 to sage-3.2.1
  • Summary changed from [with patch, needs review] make polynomial truncation cpdef method to [with patch, needs work] make polynomial truncation cpdef method

This patch causes the following doctest failures:

	sage -t -long devel/sage/sage/schemes/elliptic_curves/padics.py # 1 doctests failed
	sage -t -long devel/sage/sage/rings/power_series_ring_element.pyx # 2 doctests failed
	sage -t -long devel/sage/sage/rings/power_series_poly.pyx # 2 doctests failed
	sage -t -long devel/sage/sage/modular/modform/theta.py # 1 doctests failed
	sage -t -long devel/sage/sage/modular/modform/j_invariant.py # 1 doctests failed
	sage -t -long devel/sage/sage/crypto/lfsr.py # 5 doctests failed

The error seems to always be

Exception raised:
    Traceback (most recent call last):
      File "/scratch/mabshoff/release-cycle/sage-3.2.alpha2/local/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest __main__.example_11[3]>", line 8, in <module>
        g = Rx(g, len(g))
      File "/scratch/mabshoff/release-cycle/sage-3.2.alpha2/local/lib/python2.5/site-packages/sage/rings/power_series_ring.py", line 326, in __call__
        return self.__power_series_class(self, f, prec, check=check)
      File "power_series_poly.pyx", line 47, in sage.rings.power_series_poly.PowerSeries_poly.__init__ (sage/rings/power_series_poly.c:2073)
      File "polynomial_element.pyx", line 3928, in sage.rings.polynomial.polynomial_element.Polynomial.truncate (sage/rings/polynomial/polynomial_element.c:25338)
      File "polynomial_gf2x.pyx", line 43, in sage.rings.polynomial.polynomial_gf2x.Polynomial_GF2X.__getitem__ (sage/rings/polynomial/polynomial_gf2x.cpp:6652)
    TypeError: an integer is required

Cheers,

Michael

comment:4 follow-up: Changed 14 years ago by robertwb

Are you sure this is with my patch? I just tried these and they work fine in my branch. Or maybe it's some incompatibility with your alpha.

comment:5 in reply to: ↑ 4 Changed 14 years ago by mabshoff

Replying to robertwb:

Are you sure this is with my patch? I just tried these and they work fine in my branch. Or maybe it's some incompatibility with your alpha.

Yes, I tried with this and the patch at #2462 and initially suspected #2462, but it turns out to be this patch. Reverting this patch only fixed the issue for me. 3.2.alpha2 is coming today, so there should be a binary for sage.math shortly.

Cheers,

Michael

comment:6 Changed 14 years ago by robertwb

OK, I'll look at it there.

comment:7 Changed 14 years ago by robertwb

I tried fixing this on sage.math, but I'm having issues with the unpacked tar. I copied over sage-3.2.alpha2-sage.math-only-x86_64-Linux and extracted it, but when I run ./sage I get

sage: sage.all.__file__
 '/scratch/mabshoff/release-cycle/sage-3.2.alpha2/local/lib/python2.5/site-packages/sage/all.pyc'

and I can't figure out how test my changes. However, I'm pretty sure the error is because line 467 of sage/rings/polynomial/polynomial_template.pxi is still def (rather than cpdef). I'm attaching a patch that should fix the problem.

Changed 14 years ago by robertwb

comment:8 Changed 14 years ago by robertwb

  • Summary changed from [with patch, needs work] make polynomial truncation cpdef method to [with patch, needs review] make polynomial truncation cpdef method

comment:9 Changed 14 years ago by mabshoff

I will test the patch and see if that fixes it. More than likely you are getting bitten by #4317, so following the instructions there you should be able to fix the problem.

Cheers,

Michael

comment:10 Changed 14 years ago by mabshoff

  • Milestone changed from sage-3.2.1 to sage-3.2
  • Summary changed from [with patch, needs review] make polynomial truncation cpdef method to [with patch, positive review] make polynomial truncation cpdef method

The second patch Robert posted resolves the issue I found. Positive review.

Cheers,

Michael

comment:11 Changed 14 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.2.alpha3

Note: See TracTickets for help on using tickets.