Opened 6 years ago

Closed 6 years ago

#15306 closed defect (fixed)

partial_fraction_decomposition over QQ[] wrong

Reported by: pong Owned by:
Priority: major Milestone: sage-6.2
Component: algebra Keywords:
Cc: robertwb, dkrenn, rws Merged in:
Authors: Robert Bradshaw, Marc Mezzarobba Reviewers: Marc Mezzarobba, Robert Bradshaw
Report Upstream: N/A Work issues:
Branch: 5f4a857 (Commits) Commit: 5f4a8579c7453dc3c2de8ecae9b455ab36c47f64
Dependencies: Stopgaps:

Description

The method partial_fraction_decomposition() does not work correctly.

Compare this

sage: p = 2*x; q = x^2 + 6*x +9; r = p/q 
sage: r.partial_fraction() 
Out[2]: 2/(x + 3) - 6/(x + 3)^2 

with

sage: R.<x> = PolynomialRing(QQ)
sage: p = 2*x; q = x^2 + 6*x +9; r = p/q
sage: r.partial_fraction_decomposition()
Out[5]: (0, [2*x/(x^2 + 6*x + 9)])

Change History (18)

comment:1 Changed 6 years ago by dkrenn

  • Cc dkrenn added

comment:2 Changed 6 years ago by dkrenn

  • Summary changed from partial_fraction v.s. partial_fraction_decomposition to partial_fraction_decomposition wrong

comment:3 Changed 6 years ago by dkrenn

  • Summary changed from partial_fraction_decomposition wrong to partial_fraction_decomposition over QQ[] wrong

comment:4 Changed 6 years ago by rws

  • Cc rws added

comment:5 Changed 6 years ago by rws

  • Status changed from new to needs_info

Can you please elaborate on what output is expected? The output is correct in ZZ, and I don't see it different in QQ.

comment:6 Changed 6 years ago by pong

Hum... I don't understand, shouldn't the output of r.partial_fraction() and r.partial_fraction_decomposition() be "equivalent"? They are not in the example that I gave. For the rational function 2x/(x2 +6*x +9), the method partial_fraction() gives the correct answer while partial_fraction_decomposition() does not.

comment:7 Changed 6 years ago by mmezzarobba

  • Cc robertwb added; Robert Bradshaw removed

It looks like partial_fraction_decomposition does not even attempt to return numerators of degree smaller than that of the irreducible polynomial in the denominator. The reason for that (I guess) is that partial_fraction_decomposition is defined for all elements of parents belonging to QuotientFields(), "the category of quotient fields over an integral domain".

But while trying to fix the issue I realized that I am a bit confused:

  • Is partial fraction decomposition really defined over the field of fractions of any integral domain R? (I agree that it is when R is a PID, but what about other cases?)
  • In any case, isn't its computation going to require a Euclidean domain?
  • But in the Euclidean case, the stronger partial fraction decomposition pong asks for is always defined, isn't it?
  • What is the correct way to define methods that apply to all elements of the field of fractions of a Euclidean domain? Conversely, should there be a category of quotient fields over an integral (as opposed to Euclidean) domain in Sage at all?...

comment:8 Changed 6 years ago by rws

Please ignore my previous comment.

comment:9 Changed 6 years ago by robertwb

  • Branch set to u/robertwb/ticket/15306
  • Created changed from 10/19/13 14:52:12 to 10/19/13 14:52:12
  • Modified changed from 01/27/14 18:15:37 to 01/27/14 18:15:37

comment:10 Changed 6 years ago by git

  • Commit set to 18528939b11702517e6240eaa15ffad5052116c2

Branch pushed to git repo; I updated commit sha1. New commits:

1852893Larger example.

comment:11 follow-up: Changed 6 years ago by robertwb

  • Status changed from needs_info to needs_review

I've updated the implementation of partial_fraction_decomposition to do the full decomposition into prime powers. I also uncovered and fixed a bug in the case of non-unqiue remainders satisfying the Euclidean function inequality (e.g. ZZ).

comment:12 Changed 6 years ago by git

  • Commit changed from 18528939b11702517e6240eaa15ffad5052116c2 to 6c18589378d45119e4ab2545f0cda7ea92e9d297

Branch pushed to git repo; I updated commit sha1. New commits:

5f8be3fAdd optional argument to fraction decomposition, more docs.
6c18589Fix doctests (they're equivalent to the previous output).

comment:13 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:14 Changed 6 years ago by mmezzarobba

  • Branch changed from u/robertwb/ticket/15306 to u/mmezzarobba/15306-partial_fraction
  • Commit changed from 6c18589378d45119e4ab2545f0cda7ea92e9d297 to 5f4a8579c7453dc3c2de8ecae9b455ab36c47f64

New commits:

5f4a857#15306 rev. patch: simplifications, more tests

comment:15 in reply to: ↑ 11 Changed 6 years ago by mmezzarobba

Replying to robertwb:

I've updated the implementation of partial_fraction_decomposition to do the full decomposition into prime powers. I also uncovered and fixed a bug in the case of non-unqiue remainders satisfying the Euclidean function inequality (e.g. ZZ).

Positive review to your changes. (They do not answer my metaphysical questions, but they do fix the bug.) I prepared a small reviewer patch, though: can you have a quick look? Thanks.

comment:16 Changed 6 years ago by robertwb

  • Status changed from needs_review to positive_review

Your patch looks fine.

comment:17 Changed 6 years ago by mmezzarobba

  • Authors set to Robert Bradshaw, Marc Mezzarobba
  • Reviewers set to Marc Mezzarobba, Robert Bradshaw

comment:18 Changed 6 years ago by vbraun

  • Branch changed from u/mmezzarobba/15306-partial_fraction to 5f4a8579c7453dc3c2de8ecae9b455ab36c47f64
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.