-
-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
functions numerator() and denominator() for multivariate polynomials #6496
Comments
comment:1
A quick comment: denominator should return the right "1", I guess the one in the base ring |
Author: lftabera |
comment:2
I sent the original patch. I have realized that SAGE does neither implement numerator and denominator for univariate polynomials in general. I have open a ticket with patches in #6541 so maybe is better to wait until that patch is accepted to review this one. I have written a new patch trying to be compatible to the univariate case. a new function denominator in MPolynomial: 1.- First it tries to compute the lcm of the denominators of the coefficients. Returns this computation as result. 2.- If it cannot compute the previous denominator, returns a generic denominator. self.parent().one_element() a new function numerator in MPolynomial: returns self * self.denominator() In the univariate case there is a special case, the numerator of a polynomial with rational coefficients QQ['x'] in a polynomial with integer coefficients ZZ['x'] So I have added another patch to MPolynomial_libsingular. 3.- If the base ring of the polynomial is the field of rational numbers, the numerator is coerced to the ring of multivariate polynomials over the integers, with the same variables and same ordering as self.parent()
|
Changed author from lftabera to Luis Felipe Tabera |
comment:4
Could you attach your new patch instead of inlining it? |
comment:5
Attachment: 13222.patch.gz Tested with sage 4.2 |
Attachment: multi-poly-denom-referee.patch.gz apply on top of other |
comment:6
Looks good and passes tests. I added one tiny change--the denominator should probably live in the basering, not the parent. Other than that, positive review (which can be set as soon as my patch is looked at). |
Reviewer: Robert Bradshaw, Florent Hivert |
comment:7
Replying to @robertwb:
Done ! Everything alright (That was an easy one) => Positive review. Florent |
Merged: sage-4.3.alpha1 |
This issue was raised in sage-devel:
The person who reported the bug also attached a patch to fix this:
Component: algebra
Keywords: numerator, denominator, multivariate polynomials
Author: Luis Felipe Tabera
Reviewer: Robert Bradshaw, Florent Hivert
Merged: sage-4.3.alpha1
Issue created by migration from https://trac.sagemath.org/ticket/6496
The text was updated successfully, but these errors were encountered: