-
Notifications
You must be signed in to change notification settings - Fork 11
Semialgebraic complex plot and other plotting methods #83
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
base: master
Are you sure you want to change the base?
Semialgebraic complex plot and other plotting methods #83
Conversation
fix plotting method in semialgebraic complex with concern to broken z order of parametric plot and region plot. added animated feature to semialgebraic complex plot
| super(BasicSemialgebraicSet_section, self).__init__(base_ring=base_ring, ambient_dim=ambient_dim, names=names, poly_ring=poly_ring) | ||
| self._upstairs_bsa = upstairs_bsa | ||
| self._polynomial_map = polynomial_map | ||
|
|
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.
keep this empty line
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.
Changed.
| gcd_c = gcd(gcd(coeff), c.inhomogeneous_term()) | ||
| t = sum(QQ(x)/gcd_c*y for x, y in zip(coeff, self.poly_ring().gens())) + QQ(c.inhomogeneous_term())/gcd_c | ||
| yield t | ||
| ## add tests |
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.
Why was this change (test for is_empty) necessary again? Is something wrong with the minimized_constraints?
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 remember exactly. I think it was there as a fix from before we knew that BasicSemialgebraicSet_section wasn't stable and when we had fixed that. I've done a few quick tests and it seems like the test for emptiness is not longer necessary.
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.
Let's back out this change then
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've backed out those changes.
update plot method to use bsa that can prove emptiness fixing a plotting bug where semialgebraic complex would try to plot an empty set update NCC polyhedron to fix type error that happened when plot method was trying to plot an empty set.
40c58a9 to
1bdb425
Compare
|
please remove the ipynb files from the commits |
4f1355e to
1bdb425
Compare
|
I rolled back the changes that I had accidentally introduced. I forgot that I had done this until now. |
added animated flag to semialgebraic complex plot
fixed bugs with semialgebraic complex plot