Opened 13 years ago
Closed 13 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: | John 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 13 years ago by
Attachment: | trac7868_20100107.patch added |
---|
comment:1 Changed 13 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 13 years ago by
Owner: | changed from Alex Ghitza to spancratz |
---|
comment:4 Changed 13 years ago by
Status: | positive_review → needs_work |
---|
Well, the code looks good, but we need a test showing the behavior is fixed.
comment:5 Changed 13 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 13 years ago by
Status: | needs_work → 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 13 years ago by
Status: | needs_review → needs_work |
---|
OOps, that second patch is empty!
comment:9 Changed 13 years ago by
Status: | needs_work → needs_review |
---|
comment:10 follow-up: 11 Changed 13 years ago by
Status: | needs_review → 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 Changed 13 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 13 years ago by
Status: | positive_review → 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 13 years ago by
Authors: | spancratz → Sebastian Pancratz |
---|---|
Reviewers: | → John Cremona |
Status: | needs_work → needs_review |
comment:14 Changed 13 years ago by
Status: | needs_review → positive_review |
---|
comment:15 Changed 13 years ago by
Merged in: | → sage-4.3.1.rc2 |
---|---|
Resolution: | → fixed |
Status: | positive_review → 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