Skip to content
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

factoring broken in 0 variable polynomial ring #7950

Closed
burcin opened this issue Jan 16, 2010 · 11 comments
Closed

factoring broken in 0 variable polynomial ring #7950

burcin opened this issue Jan 16, 2010 · 11 comments

Comments

@burcin
Copy link

burcin commented Jan 16, 2010

sage: P = PolynomialRing(QQ,0,'')
sage: P
Multivariate Polynomial Ring in no variables over Rational Field
sage: t = P.random_element()
sage: t.factor()
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)

/home/burcin/.sage/temp/karr/24426/_home_burcin__sage_init_sage_0.py in <module>()

/home/burcin/sage/sage-4.3.alpha0/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_element.pyc in factor(self, proof)
   1422         # try to use univariate factoring first

   1423         try:
-> 1424             F = self.univariate_polynomial().factor()
   1425             return Factorization([(R(f),m) for f,m in F], unit=F.unit())
   1426         except TypeError:

/home/burcin/sage/sage-4.3.alpha0/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_element.pyc in univariate_polynomial(self, R)
   1055         #construct ring if None

   1056         if R is None:
-> 1057             R = self.base_ring()[str(self.variables()[0])]
   1058 
   1059         monomial_coefficients = self._MPolynomial_element__element.dict()

IndexError: tuple index out of range

Component: commutative algebra

Author: Burcin Erocal

Reviewer: William Stein, Alex Ghitza

Merged: sage-4.3.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7950

@burcin
Copy link
Author

burcin commented Jan 17, 2010

factor zero variable polynomials

@burcin
Copy link
Author

burcin commented Jan 17, 2010

comment:1

Attachment: trac_7950-zero_variable_factor.patch.gz

trivial review please?

@burcin
Copy link
Author

burcin commented Jan 17, 2010

Author: Burcin Erocal

@williamstein
Copy link
Contributor

comment:2
  1. Factoring of 0 should raise an error like it does over ZZ, but doesn't right now:
sage: P = PolynomialRing(ZZ,0,'')
sage: P(10).factor()
10
sage: P(0).factor()
0
sage: factor(0)
---------------------------------------------------------------------------
ArithmeticError                           Traceback (most recent call last)
  1. The element 10 in the polynomial ring "ZZ[]" in 0-variables is actually not a unit. So it is wrong that it is put in the "unit" slot of the factorization. Notice how factoring 10 works:
sage: R.<x> = ZZ[]
sage: (10*x).factor()
2 * 5 * x
sage: list((10*x).factor())
[(2, 1), (5, 1), (x, 1)]

In particular, the 10 is not treated incorrectly as a unit.

So I think this patch needs work.

@burcin
Copy link
Author

burcin commented Jan 18, 2010

Attachment: trac_7950-zero_variable_factor.take2.patch.gz

only apply this patch

@burcin
Copy link
Author

burcin commented Jan 18, 2010

Reviewer: William Stein

@burcin
Copy link
Author

burcin commented Jan 18, 2010

comment:3

Thanks for the review!

New patch addressing both points is available at attachment: trac_7950-zero_variable_factor.take2.patchI hope it doesn't contain more stupid mistakes. :)

@aghitza
Copy link

aghitza commented Jan 23, 2010

Changed reviewer from William Stein to William Stein, Alex Ghitza

@aghitza
Copy link

aghitza commented Jan 23, 2010

comment:4

Replying to @burcin:

New patch addressing both points is available at attachment: trac_7950-zero_variable_factor.take2.patchI hope it doesn't contain more stupid mistakes. :)

Not that I could find :)

Looks good to me.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 23, 2010

comment:5

Merged trac_7950-zero_variable_factor.take2.patch.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 23, 2010

Merged: sage-4.3.2.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Jan 23, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants