Opened 9 years ago

Closed 3 years ago

#12129 closed defect (wontfix)

MPolynomial_libsingular.is_squarefree fails with linear factors

Reported by: culler Owned by: AlexGhitza
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: algebra Keywords: square free
Cc: jakobkroeker Merged in:
Authors: Paul Zimmermann, John Cremona Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: u/cremona/12129 (Commits, GitHub, GitLab) Commit: 17fdcc12d73b82446e8122cb3e0de3fbcb1671ba
Dependencies: Stopgaps: todo

Status badges

Description 

sage: x, y = QQ['x','y'].gens()
sage: f = x*y
sage: f.is_squarefree()
False
sage: f = (x+y)*(x+3)
sage: f.is_squarefree()
False

This may be related to an inconsistency in the Singular sqrfree function, which sometimes includes a unit in the square free factorization, but other times does not.

Change History (17)

comment:1 Changed 9 years ago by zimmerma

see also #12198 and #12404. Maybe some of those tickets can be removed as duplicate.

Paul

comment:2 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:6 Changed 6 years ago by jakobkroeker

  • Cc jakobkroeker added

comment:7 Changed 6 years ago by jakobkroeker

  • Stopgaps set to todo

comment:8 Changed 3 years ago by cremona

This can surely be closed as the examples posted are now computed correctly. Same as at #12198.

comment:9 Changed 3 years ago by tscrim

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Reviewers set to Travis Scrimshaw
  • Status changed from new to needs_review

comment:10 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:11 Changed 3 years ago by zimmerma

shouldn't we include a non-regression test?

comment:12 Changed 3 years ago by tscrim

I am not opposed. Feel free to use this ticket for that.

comment:13 Changed 3 years ago by zimmerma

here is a corresponding patch: 

--- a/src/sage/rings/polynomial/multi_polynomial_libsingular.pyx
+++ b/src/sage/rings/polynomial/multi_polynomial_libsingular.pyx
@@ -4734,6 +4734,18 @@ cdef class MPolynomial_libsingular(MPolynomial):
             sage: h = f^2
             sage: h.is_squarefree()
             False
+
+        TESTS::
+
+Check if :trac:`12129` is fixed::
+
+            sage: x, y = QQ['x','y'].gens()
+            sage: f = x*y
+            sage: f.is_squarefree()
+            True
+            sage: f = (x+y)*(x+3)
+            sage: f.is_squarefree()
+            True
         """
         # TODO:  Use Singular (4.x) intrinsics.  (Temporary solution from #17254.)
         return all([ e == 1 for (f, e) in self.factor() ])

comment:14 Changed 3 years ago by cremona

I agree that tests should be added, both here and at #12198, but was a little too lazy to do it myself.

comment:15 Changed 3 years ago by cremona

  • Authors set to Paul Zimmermann, John Cremona
  • Branch set to u/cremona/12129
  • Commit set to 17fdcc12d73b82446e8122cb3e0de3fbcb1671ba
  • Status changed from positive_review to needs_review

Travis could you re-review this? I thought that this doctest would do for both tickets. Thanks.

New commits:

​17fdcc1#12129: add doctest to show issue has been fixed

comment:16 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

I was also a little too lazy to do it as well. ^^;; Thanks Paul and John.

Last edited 3 years ago by tscrim (previous) (diff)

comment:17 Changed 3 years ago by embray

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.