-
-
Notifications
You must be signed in to change notification settings - Fork 528
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
roots of polynomial have incorrect parent when ring=R is specified #6207
Comments
comment:1
Attachment: roots-parent-2.patch.gz |
comment:2
I don't think this was really meant for review. Nick's patch just demonstrates that the function |
comment:3
Attachment: trac_6207.patch.gz Apply only the patch I posted. Note -- with the first patch the checks about parents are left on, so the referee can run the test suite with them on. The second part2 patch turns them off. NOTE: I did not fix the issues with precision being too hire for CIF and RIF, since that behavior is already very clearly documented (IMHO) by Carl Witty in the roots method. In part2 I get rid of all the debug code. My reasoning is: (1) I want the roots docstring to be on that method. (2) The code was really specifically for debugging this problem, which turned out to be mostly caused by my non-uniqueness of the RealField object. By the way, the main work of this patch is to make it so RealField caching and naming is more sensible, systematic, correct, and the same as is already done for ComplexField. |
Attachment: trac_6207-part2.patch.gz |
comment:5
I applied
|
comment:6
In
makes Sage build ok. Is this the right fix? I'm a bit thrown off by that file starting with the line I'm running tests now. |
comment:7
After reading William's first patch more carefully, I now know that the fix needs to be made in I'll attach a reviewer patch in a second, and run tests. |
apply after the previous two patches |
comment:8
Attachment: trac_6207-review.patch.gz OK, this looks good. I give a positive review to William's two patches. It remains for someone to look at my reviewer patch -- this should be very quick now. Note that, apart from fixing the build, I made the comment at the top of the interpreter files a little more explicit. |
Reviewer: Alex Ghitza |
Author: William Stein |
comment:10
I approve of the reviewer patch. |
Merged: sage-4.3.2.alpha0 |
comment:12
Merged in this order: |
The attached patch verifies that the roots of a polynomial actually have parent the desired ring. This bit me in the ass for number fields, but the setup required bizarre embeddings and I don't have an example at hand. This is not true in a few cases over inexact fields:
CC: cwitty mhansen
Component: algebra
Keywords: roots polynomial parent ring inexact
Author: William Stein
Reviewer: Alex Ghitza
Merged: sage-4.3.2.alpha0
Issue created by migration from https://trac.sagemath.org/ticket/6207
The text was updated successfully, but these errors were encountered: