Ticket #12404 (closed defect: fixed)
is_squarefree() incorrect over imperfect fields
| Reported by: | saraedum | Owned by: | malb |
|---|---|---|---|
| Priority: | minor | Milestone: | sage-5.1 |
| Component: | commutative algebra | Keywords: | sd40.5 |
| Cc: | sydahmad | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Paul Zimmermann, Jeroen Demeyer |
| Authors: | Julian Rueth | Merged in: | sage-5.1.beta3 |
| Dependencies: | #9054,#13043, #12988, #10902 | Stopgaps: |
Description (last modified by jdemeyer) (diff)
The method is_squarefree() is incorrect for polynomials over function fields over finite fields:
sage: K.<x> = FunctionField(GF(3)) sage: R.<T> = K[] sage: f=T^3-x sage: f.factor(proof=False) T^3 + 2*x sage: f.is_squarefree() False
Apply:
to the sage library.
Attachments
Change History
comment:2 in reply to: ↑ 1 Changed 17 months ago by saraedum
Replying to saraedum:
True in [d>=2 for (f,d) in f.factor(proof=False)]
This would be "is_not_squarefree()" of course.
comment:5 Changed 13 months ago by saraedum
Thanks for pointing these out. Since both are in the multivariate case, I don't think they're related.
comment:6 Changed 13 months ago by saraedum
- Status changed from new to needs_review
- Dependencies changed from #9054 to #9054, #12404
- Authors set to Julian Rueth
The attached patch should fix the problem.
comment:7 Changed 13 months ago by saraedum
- Dependencies changed from #9054, #12404 to #9054, #12988
comment:9 follow-up: ↓ 10 Changed 13 months ago by zimmerma
- Status changed from needs_review to needs_work
- Reviewers set to Paul Zimmermann
- Work issues set to coherence with squarefree_decomposition, fix error
the documentation of is_squarefree says that f is not square-free if g^2 divides f where g is a non-unit. In particular 4*x is not considered square free by is_squarefree, but it is by squarefree_decomposition:
sage: R.<x> = ZZ[] sage: f = 4*x sage: f.is_squarefree() False sage: f.squarefree_decomposition() (4) * x
Sage should be coherent in that matter. Personally I prefer not to decompose the coefficient content.
Moreover the following produces an error:
sage: R.<x> = ZZ[] sage: f = 2*x^2 sage: f.is_squarefree() ... AttributeError: 'int' object has no attribute 'is_zero'
Paul
comment:10 in reply to: ↑ 9 Changed 13 months ago by saraedum
Replying to zimmerma:
the documentation of is_squarefree says that f is not square-free if g^2 divides f where g is a non-unit. In particular 4*x is not considered square free by is_squarefree, but it is by squarefree_decomposition:
sage: R.<x> = ZZ[] sage: f = 4*x sage: f.is_squarefree() False sage: f.squarefree_decomposition() (4) * xSage should be coherent in that matter. Personally I prefer not to decompose the coefficient content.
That is true. I also noted this inconsistency. To not break existing code, I'd rather add a warning section in the docstring. Generally I agree that having squarefree_decomposition() factor the content is not what one wants for most purposes.
Would you be ok with just adding a warning and an example showing this problem?
Moreover the following produces an error:
sage: R.<x> = ZZ[] sage: f = 2*x^2 sage: f.is_squarefree() ... AttributeError: 'int' object has no attribute 'is_zero'
This should not happen when applying the dependency #12988.
comment:12 Changed 13 months ago by saraedum
I added a patch with such a warning. I still have to check how it renders in the docs.
comment:13 Changed 13 months ago by saraedum
- Status changed from needs_info to needs_review
- Description modified (diff)
I made a few docstring changes in the latest patch.
Changed 13 months ago by saraedum
-
attachment
trac_12404.patch
added
distinguish characteristic zero and nonzero
comment:14 Changed 13 months ago by saraedum
Btw. I will add the respective warning for squarefree_decomposition() in #13008.
comment:15 follow-up: ↓ 17 Changed 13 months ago by zimmerma
Julian, there is a typo in trac_12404_warning.patch: not to consider to content should read not to consider the content.
Would it break a lot of code if is_squarefree returns False for 4*x? Did you try?
Paul
Changed 13 months ago by saraedum
-
attachment
trac_12404_warning.patch
added
warning about inconsistency with squarefree_decomposition
comment:16 Changed 13 months ago by saraedum
Thanks, the typo should be fixed now.
No I haven't tried. I was actually thinking about external code using that method. If you insist we can change it. I don't have a very strong opinion about this. I just believe that inconsistency isn't bad as long as it's documented. IMHO breaking the interface is worse than documented inconsistency.
comment:17 in reply to: ↑ 15 Changed 13 months ago by jdemeyer
Replying to zimmerma:
Would it break a lot of code if is_squarefree returns False for 4*x?
Did you mean "if is_squarefree returns True for 4*x"?
comment:18 Changed 13 months ago by jdemeyer
- Dependencies changed from #9054, #12988 to #9054, #12988, #10902
comment:19 Changed 13 months ago by jdemeyer
- Reviewers changed from Paul Zimmermann to Paul Zimmermann, Jeroen Demeyer
- Description modified (diff)
Positive review to the first two patches. Anybody else can review my patch?
comment:20 Changed 13 months ago by was
- Status changed from needs_review to positive_review
jdmeyer -- I positively review your patch.
comment:21 Changed 13 months ago by jdemeyer
- Work issues coherence with squarefree_decomposition, fix error deleted
- Report Upstream changed from Not yet reported upstream; Will do shortly. to N/A
comment:23 Changed 13 months ago by jdemeyer
- Dependencies changed from #9054, #12988, #10902 to #9054,#13043, #12988, #10902
comment:24 Changed 13 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-5.1.beta3

The method is_squarefree() returns self.derivative().gcd(self).degree() <= 0. In the example the derivative vanishes which leads to the incorrect behaviour.
As a quick workaround one can do: