Opened 10 years ago
Closed 10 years ago
#7868 closed defect (fixed)
Factoring in fraction fields
Reported by: | spancratz | Owned by: | spancratz |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.3.1 |
Component: | algebra | Keywords: | fraction field, factorization |
Cc: | cremona | Merged in: | sage-4.3.1.rc2 |
Authors: | Sebastian Pancratz | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The following is a quote from John Cremona,
http://groups.google.com/group/sage-devel/browse_thread/thread/3638a91c0438f439
I define a rational function in two variables over a finite field:
sage: R.<x,y> = GF(2)[] sage: f = x*y/(x+y) sage: f.parent() Fraction Field of Multivariate Polynomial Ring in x, y over Finite Field of size 2
I try to factor it, and get this error:
sage: f.factor() --------------------------------------------------------------------------- NotImplementedError Traceback (most recent call last) /home/masgaj/.sage/temp/host_56_150/17587/_home_masgaj__sage_init_sage_0.py in <module>() /local/jec/sage-4.3.rc0/local/lib/python2.6/site-packages/sage/rings/fraction_field_element.so in sage.rings.fraction_field_element.FractionFieldElement.factor (sage/rings/fraction_field_element.c:2972)() /local/jec/sage-4.3.rc0/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular.factor (sage/rings/polynomial/multi_polynomial_libsingular.cpp:22701)() NotImplementedError: proof = True factorization not implemented. Call factor with proof=False.
So I do what I am told, but:
sage: f.factor(proof=False) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) /home/masgaj/.sage/temp/host_56_150/17587/_home_masgaj__sage_init_sage_0.py in <module>() TypeError: factor() takes no keyword arguments
Attachments (4)
Change History (19)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Owner changed from AlexGhitza to spancratz
comment:3 Changed 10 years ago by
- Status changed from needs_review to positive_review
Looks good to me.
comment:4 Changed 10 years ago by
- Status changed from positive_review to needs_work
Well, the code looks good, but we need a test showing the behavior is fixed.
comment:5 Changed 10 years ago by
I agree with Robert's point, which I was going to make too. But for me the code does not apply to either 4.3 or 4.3.1.apha1. Sebastian, did you make the patch on top of your other changes to the fraction field code? Robert, did you apply the patch?
comment:6 Changed 10 years ago by
- Status changed from needs_work to needs_review
No, I didn't actually apply it... (I've got a lot of fraction field changes that I needed to pop off first, and no, it's not applying for me either. I don't see why.) I've posted an updated one that should merge onto 4.3 fine, with the new doctest.
comment:7 Changed 10 years ago by
- Status changed from needs_review to needs_work
OOps, that second patch is empty!
comment:8 Changed 10 years ago by
Oops...
comment:9 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:10 follow-up: ↓ 11 Changed 10 years ago by
- Status changed from needs_review to positive_review
I deleted the lines sage: f.parent() in the patch since you omitted its output. If this works for you: positive review. Thanks!
comment:11 in reply to: ↑ 10 Changed 10 years ago by
Great. This all happened rather swiftly!
Sebastian
Replying to cremona:
I deleted the lines sage: f.parent() in the patch since you omitted its output. If this works for you: positive review. Thanks!
comment:12 Changed 10 years ago by
- Status changed from positive_review to needs_work
Sorry, there's a conflict:
patching file sage/rings/fraction_field_element.pyx Hunk #1 FAILED at 212 1 out of 2 hunks FAILED -- saving rejects to file sage/rings/fraction_field_element.pyx.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh 7868-fix.2.patch
comment:13 Changed 10 years ago by
- Reviewers set to John Cremona
- Status changed from needs_work to needs_review
comment:14 Changed 10 years ago by
- Status changed from needs_review to positive_review
comment:15 Changed 10 years ago by
- Merged in set to sage-4.3.1.rc2
- Resolution set to fixed
- Status changed from positive_review to closed
The patch I attached earlier changes the behaviour of the factoring method in fraction fields, passing on additional parameters to the factoring method of the underlying ring rather than ignoring them.
I've now tested the patch (without the
long
option), and this did not return any test failures.
Sebastian