-
Notifications
You must be signed in to change notification settings - Fork 63
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
Fix a bug in gcd for polynomials #1799
Conversation
Can you post the example to reproduce it? |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1799 +/- ##
=======================================
Coverage 87.99% 87.99%
=======================================
Files 119 119
Lines 30073 30073
=======================================
Hits 26464 26464
Misses 3609 3609 ☔ View full report in Codecov by Sentry. |
This version (with the third boolean argument) should probably not be user-facing, but I guess there is no harm fixing it here. |
could/should we add a test? |
I don't know what the function is supposed to do, so I don't feel qualified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really follow this whole function, but c
is clearly only set if !ignore_content
holds, so the existing code can't be right. The new code seems at least plausible.
So let's merge this. If there are problems here, and if anyone is actually using this code, we'll just have to hope for a bug report.
I don't know what this function is doing, so this is an educated guess. For polynomials over fields, another
gcd
is used. The function fixed here is only called if one specifies theignore_content = true
.I found this by calling
is_squarefree
on a polynomial over a number field in Nemo.