Opened 9 years ago

Closed 9 years ago

#9945 closed defect (fixed)

partial_fraction_decomposition broken for FpT elements

Reported by: robertwb Owned by: AlexGhitza
Priority: major Milestone: sage-4.6
Component: basic arithmetic Keywords:
Cc: Merged in: sage-4.6.alpha2
Authors: Robert Bradshaw Reviewers: Paul Zimmermann
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

> sage: R.<x> = GF(3)[]
> sage: q = (x+1)/(x^3+x+1)
> sage: q.partial_fraction_decomposition()

See http://groups.google.com/group/sage-support/browse_thread/thread/5423a314227309b3#

Attachments (4)

9945-part-frac-FpT.patch (24.4 KB) - added by robertwb 9 years ago.
9945-referee-fixes.patch (2.1 KB) - added by robertwb 9 years ago.
9945-rebased.patch (25.6 KB) - added by robertwb 9 years ago.
apply only this patch
9945-rebased.2.patch (25.1 KB) - added by mpatel 9 years ago.
Rebased against #8334 which should be in 4.6.alpha2. Apply only this patch.

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by robertwb

comment:1 Changed 9 years ago by robertwb

  • Status changed from new to needs_review

comment:2 Changed 9 years ago by zimmerma

  • Status changed from needs_review to needs_work

Two small comments: in line 115 of sage/categories/quotient_fields.py, "in-exact" should be "inexact". Also I don't understand "what side effects would this have this be bad?" on line 166.

Paul

comment:3 Changed 9 years ago by zimmerma

in addition, 2 doctests fail (with 4.5.3):

The following tests failed:

        sage -t  devel/sage-9945/sage/rings/ring.pyx # 2 doctests failed
----------------------------------------------------------------------
Timings have been updated.
Total time for all tests: 5056.9 seconds

Paul

Changed 9 years ago by robertwb

comment:4 Changed 9 years ago by robertwb

  • Status changed from needs_work to needs_review

I was just moving code, but it doesn't hurt to clean it up as I do so (and I was the original author, so the criticism is just :) I removed the TODO, I was thinking about changing the factor command to group "equal" roots but that would be too invasive of a change to consider right now.

comment:5 Changed 9 years ago by zimmerma

  • Status changed from needs_review to needs_work
  • Work issues set to rebase

Robert, sorry the original patch fails to apply to 4.6.alpha1:

----------------------------------------------------------------------
| Sage Version 4.6.alpha1, Release Date: 2010-09-18                  |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
Loading Sage library. Current Mercurial branch is: 9945
sage: hg_sage.import_patch("/tmp/9945-part-frac-FpT.patch")
cd "/tmp/sage-4.6.alpha1/devel/sage" && hg status
cd "/tmp/sage-4.6.alpha1/devel/sage" && hg status
cd "/tmp/sage-4.6.alpha1/devel/sage" && hg import   "/tmp/9945-part-frac-FpT.patch"
applying /tmp/9945-part-frac-FpT.patch
patching file sage/rings/fraction_field_element.pyx
Hunk #3 FAILED at 282
1 out of 4 hunks FAILED -- saving rejects to file sage/rings/fraction_field_element.pyx.rej
abort: patch failed to apply

Please could you rebase it?

Paul

Changed 9 years ago by robertwb

apply only this patch

comment:6 Changed 9 years ago by robertwb

  • Status changed from needs_work to needs_review

comment:7 Changed 9 years ago by zimmerma

  • Authors set to Robert Bradshaw
  • Reviewers set to Paul Zimmermann
  • Status changed from needs_review to positive_review
  • Work issues rebase deleted

The rebased patch works ok with 4.6.alpha1.

Changed 9 years ago by mpatel

Rebased against #8334 which should be in 4.6.alpha2. Apply only this patch.

comment:8 Changed 9 years ago by mpatel

I've attached a patch rebased for the queue in merger-4.6.alpha2.

comment:9 Changed 9 years ago by mpatel

  • Merged in set to sage-4.6.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.