Opened 10 years ago
Closed 4 years ago
#6551 closed enhancement (fixed)
fix ugliness in printing of multivariate polynomials
Reported by: | AlexGhitza | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | algebra | Keywords: | print latex multivariate polynomial |
Cc: | malb | Merged in: | |
Authors: | Alex Ghitza | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The printing (and latex-ing) of multivariate polynomials is sometimes quite ugly, and inconsistent with the much prettier printing of univariate polynomials. One gets things like the following (taken from doctests in the Sage library):
(-6/5)*x^2*y^2 + (-3)*x*y^3 + 6/5*x^2*y + 11/12*x*y^2 + (-18)*y^2 + (-3/4)*y
or even
sage: xgcd((b+g)*y^2, (a+g)*y+b) ((b^3 + (g)*b^2)/(a^2 + (2*g)*a + 3), 1, ((-b + (-g))/(a + (g)))*y + (b^2 + (g)*b)/(a^2 + (2*g)*a + 3))
The attached patch fixes this, factors out common code for printing and latex-ing, and makes printing consistent across various representations of multivariate polynomials.
Attachments (3)
Change History (21)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Owner changed from tbd to AlexGhitza
- Status changed from new to assigned
- Summary changed from fix ugliness in printing of multivariate polynomials to [with patch, needs review] fix ugliness in printing of multivariate polynomials
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Here's a patch that applies cleanly on top of the #6500 patches. For what it's worth, the patch looks fine to me, but I'm not an expert on multivariate commutative algebra so probably it'd be better to get someone else to review it.
comment:4 Changed 10 years ago by
- Summary changed from [with patch, needs review] fix ugliness in printing of multivariate polynomials to [with patch, needs work] fix ugliness in printing of multivariate polynomials
Hi Alex,
I decided to run all doctests just to check that I had rebased the patch correctly, but there seems to be some funny business in quaternion algebras. This is with the #6500 patches and the rebased patch here, applied to 4.1:
sage -t "devel/sage/sage/algebras/quaternion_algebra_element.py" ********************************************************************** File "/home/david/sage-4.1/devel/sage/sage/algebras/quaternion_algebra_element.py", line 17: sage: sage.algebras.quaternion_algebra_element.unpickle_QuaternionAlgebraElement_generic_v0(*t) Expected: 2/3 + X*i - X^2*j + X^3*k Got: 2/3 + X*i + (-X^2)*j + X^3*k **********************************************************************
and
********************************************************************** File "/home/david/sage-4.1/devel/sage/sage/algebras/quatalg/quaternion_algebra.py", line 455: sage: QuaternionAlgebra(GF(17)(2),3).random_element() Expected: 11 + 16*i + 4*j + 13*k Got: 11 - i + 4*j + 13*k **********************************************************************
This has nothing to do with the rebasing, because I ran these two tests again using your original patch and without the #6500 patches and they failed in exactly the same way. Any idea what is going on here?
David
comment:5 Changed 10 years ago by
- Reviewers set to David Loeffler
comment:6 Changed 10 years ago by
Hi David,
I just checked and I'm getting the same failures as you both on my laptop and sage.math (which is weird because I could swear I tested this about a million times).
I have an inkling of what might be going on but it's going to have to wait until tomorrow because I'm falling asleep in my chair.
Thanks for reviewing, and for the work you put in rebasing the patch.
Changed 10 years ago by
comment:7 Changed 10 years ago by
- Summary changed from [with patch, needs work] fix ugliness in printing of multivariate polynomials to [with patch, needs review] fix ugliness in printing of multivariate polynomials
Yes, the problem was indeed that I was mixing up some stuff with #6183.
I've added a small patch that takes care of this, and all tests (should) now pass.
comment:8 Changed 10 years ago by
- Summary changed from [with patch, needs review] fix ugliness in printing of multivariate polynomials to [with patch, needs work] fix ugliness in printing of multivariate polynomials
Unfortunately, the patch has bit-rotted:
applying trac_6551-rebased_for_6500.patch patching file sage/matrix/matrix_mpolynomial_dense.pyx Hunk #1 FAILED at 0 1 out of 6 hunks FAILED -- saving rejects to file sage/matrix/matrix_mpolynomial_dense.pyx.rej patching file sage/rings/polynomial/polydict.pyx Hunk #5 succeeded at 887 with fuzz 1 (offset 0 lines). patching file sage/rings/polynomial/toy_buchberger.py Hunk #1 FAILED at 53 1 out of 1 hunks FAILED -- saving rejects to file sage/rings/polynomial/toy_buchberger.py.rej patching file sage/schemes/elliptic_curves/ell_curve_isogeny.py Hunk #7 FAILED at 1864 1 out of 21 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/ell_curve_isogeny.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir Errors during apply, please fix and refresh trac_6551-rebased_for_6500.patch
I read the patch and it looks fine so far. It is mainly a question of taste anyway IMHO. However, it might be worth checking for performance loses due to this patch (conversion to strings is used to communicate with Singular for instance)
comment:9 Changed 10 years ago by
After thinking about this some more time: I think the proposed implementation for libSingular polynomials is way too slow. I suggest someone (e.g. me) re-implements the PolyDict? printing for those to make it reasonably efficient.
comment:10 Changed 10 years ago by
Martin,
Thanks for looking at this carefully, and sorry for having left things somewhat half-baked. In my defense, I had already spent a long time getting the formatting to be consistent, and I'm not sure I'd be very good at speed issues. But I completely agree with you that this is ubiquitous code that should be fast. Count on me to referee your implementation when you're done with it.
comment:11 Changed 10 years ago by
- Summary changed from [with patch, needs work] fix ugliness in printing of multivariate polynomials to fix ugliness in printing of multivariate polynomials
comment:12 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:13 Changed 6 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:14 Changed 6 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:15 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:16 Changed 4 years ago by
- Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
- Report Upstream set to N/A
This ticket seems invalid. For the first example:
sage: R.<x,y> = QQ[] sage: p = (-6/5)*x^2*y^2 + (-3)*x*y^3 + 6/5*x^2*y + 11/12*x*y^2 + (-18)*y^2 + (-3/4)*y sage: p -6/5*x^2*y^2 - 3*x*y^3 + 6/5*x^2*y + 11/12*x*y^2 - 18*y^2 - 3/4*y
I cannot make the second example work (and this does not appear in the doctests).
comment:17 Changed 4 years ago by
- Status changed from needs_work to positive_review
comment:18 Changed 4 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
Bad news: this conflicts quite severely with my patches at #6500. I feel a bit guilty about this, because it was my referee comments on #6183 that pushed you into working on this, so I will handle preparing a rebased version.