Opened 10 years ago
Closed 10 years ago
#5928 closed defect (fixed)
[with patch, positive review] binary operations on factorisations should coerce factors into a common universe
Reported by: | AlexGhitza | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-3.4.2 |
Component: | factorization | Keywords: | multiplication factorization coercion |
Cc: | cremona | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This was uncovered at #5921. Observe:
sage: P.<x> = ZZ sage: f = 2*x + 2 sage: c = f.content() sage: g = f//c sage: F1 = c.factor(); [type(a[0]) for a in F1] [<type 'sage.rings.integer.Integer'>] sage: F2 = g.factor(); [type(a[0]) for a in F2] [<type 'sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint'>] sage: F1*F2 2 * (x + 1) sage: [type(a[0]) for a in F1*F2] [<type 'sage.rings.integer.Integer'>, <type 'sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint'>]
I think that multiplying two factorisations should make sure that the factors can be coerced into a common universe, so that all factors have the same parent. If that's impossible, then an error should be thrown.
Attachments (1)
Change History (4)
comment:1 Changed 10 years ago by
- Status changed from new to assigned
- Summary changed from multiplication of factorisations should coerce factors into a common universe to [with patch, needs review] binary operations on factorisations should coerce factors into a common universe
Changed 10 years ago by
comment:2 Changed 10 years ago by
- Summary changed from [with patch, needs review] binary operations on factorisations should coerce factors into a common universe to [with patch, positive review] binary operations on factorisations should coerce factors into a common universe
This patch looks good, applies to 3.4.2.alpha0 and tests in sage/structure pass as well as those in sage/rings/*.py (I did not go into subdirectories).
I was a little disappointed by this:
sage: R.<x> = ZZ[] sage: S.<y> = QQ[] sage: f = x^2-1 sage: g = y^3-1 sage: f.factor() (x - 1) * (x + 1) sage: g.factor() (y - 1) * (y^2 + y + 1) sage: f.factor() * g.factor() (1) * (y - 1) * (x - 1) * (x + 1) * (y^2 + y + 1) sage: (f.factor() * g.factor()).universe() Category of objects
and in fact coercion is not clever enough to allow x*y here. but it does work if you do
sage: S.<x> = QQ[] sage: y=S.gen(0) sage: g = y^3-1 sage: f.factor() * g.factor() (x + 1) * (x - 1)^2 * (x^2 + x + 1)
-- i.e. you have to define the two rings with the same name of the variable even if you use a different name for input. Weird, but it is not going to stop this patch!
comment:3 Changed 10 years ago by
- Milestone changed from sage-4.0 to sage-3.4.2
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.4.2.rc0.
Cheers,
Michael
The attached patch does this for multiplication, gcd, and lcm. The other operations either inherit it or were dealing with this already.