| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -1510,20 +1510,35 @@ def _calc_coord(self, xv, yv, renderer=None): | |
|
|
||
| def _arcball(self, x: float, y: float) -> np.ndarray: | ||
| """ | ||
| Convert a point (x, y) to a point on a virtual trackball | ||
| This is Ken Shoemake's arcball | ||
| Convert a point (x, y) to a point on a virtual trackball. | ||
|
|
||
| This is Ken Shoemake's arcball (a sphere), modified | ||
| to soften the abrupt edge (optionally). | ||
| See: Ken Shoemake, "ARCBALL: A user interface for specifying | ||
| three-dimensional rotation using a mouse." in | ||
| Proceedings of Graphics Interface '92, 1992, pp. 151-156, | ||
| https://doi.org/10.20380/GI1992.18 | ||
| """ | ||
| x *= 2 | ||
| y *= 2 | ||
| The smoothing of the edge is inspired by Gavin Bell's arcball | ||
| (a sphere combined with a hyperbola), but here, the sphere | ||
| is combined with a section of a cylinder, so it has finite support. | ||
| """ | ||
| s = mpl.rcParams['axes3d.trackballsize'] / 2 | ||
| b = mpl.rcParams['axes3d.trackballborder'] / s | ||
| x /= s | ||
| y /= s | ||
| r2 = x*x + y*y | ||
| if r2 > 1: | ||
| p = np.array([0, x/math.sqrt(r2), y/math.sqrt(r2)]) | ||
| r = np.sqrt(r2) | ||
| ra = 1 + b | ||
| a = b * (1 + b/2) | ||
| ri = 2/(ra + 1/ra) | ||
| if r < ri: | ||
| p = np.array([np.sqrt(1 - r2), x, y]) | ||
| elif r < ra: | ||
| dr = ra - r | ||
| p = np.array([a - np.sqrt((a + dr) * (a - dr)), x, y]) | ||
| p /= np.linalg.norm(p) | ||
| else: | ||
| p = np.array([math.sqrt(1-r2), x, y]) | ||
| p = np.array([0, x/r, y/r]) | ||
| return p | ||
|
|
||
| def _on_move(self, event): | ||
| Expand Down Expand Up | @@ -1561,23 +1576,35 @@ def _on_move(self, event): | |
| if dx == 0 and dy == 0: | ||
| return | ||
|
|
||
| # Convert to quaternion | ||
| elev = np.deg2rad(self.elev) | ||
| azim = np.deg2rad(self.azim) | ||
| roll = np.deg2rad(self.roll) | ||
| q = _Quaternion.from_cardan_angles(elev, azim, roll) | ||
|
|
||
| # Update quaternion - a variation on Ken Shoemake's ARCBALL | ||
| current_vec = self._arcball(self._sx/w, self._sy/h) | ||
| new_vec = self._arcball(x/w, y/h) | ||
| dq = _Quaternion.rotate_from_to(current_vec, new_vec) | ||
| q = dq * q | ||
|
|
||
| # Convert to elev, azim, roll | ||
| elev, azim, roll = q.as_cardan_angles() | ||
| azim = np.rad2deg(azim) | ||
| elev = np.rad2deg(elev) | ||
| roll = np.rad2deg(roll) | ||
| style = mpl.rcParams['axes3d.mouserotationstyle'] | ||
| if style == 'azel': | ||
|
Comment thread
scottshambaugh marked this conversation as resolved.
Comment thread
scottshambaugh marked this conversation as resolved.
|
||
| roll = np.deg2rad(self.roll) | ||
| delev = -(dy/h)*180*np.cos(roll) + (dx/w)*180*np.sin(roll) | ||
| dazim = -(dy/h)*180*np.sin(roll) - (dx/w)*180*np.cos(roll) | ||
| elev = self.elev + delev | ||
| azim = self.azim + dazim | ||
| roll = self.roll | ||
| else: | ||
| q = _Quaternion.from_cardan_angles( | ||
| *np.deg2rad((self.elev, self.azim, self.roll))) | ||
|
|
||
| if style == 'trackball': | ||
| k = np.array([0, -dy/h, dx/w]) | ||
| nk = np.linalg.norm(k) | ||
| th = nk / mpl.rcParams['axes3d.trackballsize'] | ||
| dq = _Quaternion(np.cos(th), k*np.sin(th)/nk) | ||
| else: # 'sphere', 'arcball' | ||
| current_vec = self._arcball(self._sx/w, self._sy/h) | ||
| new_vec = self._arcball(x/w, y/h) | ||
| if style == 'sphere': | ||
| dq = _Quaternion.rotate_from_to(current_vec, new_vec) | ||
| else: # 'arcball' | ||
| dq = _Quaternion(0, new_vec) * _Quaternion(0, -current_vec) | ||
|
|
||
| q = dq * q | ||
| elev, azim, roll = np.rad2deg(q.as_cardan_angles()) | ||
|
|
||
| # update view | ||
| vertical_axis = self._axis_names[self._vertical_axis] | ||
| self.view_init( | ||
| elev=elev, | ||
| Expand Down Expand Up | @@ -3984,7 +4011,7 @@ def rotate_from_to(cls, r1, r2): | |
| k = np.cross(r1, r2) | ||
| nk = np.linalg.norm(k) | ||
| th = np.arctan2(nk, np.dot(r1, r2)) | ||
| th = th/2 | ||
| th /= 2 | ||
| if nk == 0: # r1 and r2 are parallel or anti-parallel | ||
| if np.dot(r1, r2) < 0: | ||
| warnings.warn("Rotation defined by anti-parallel vectors is ambiguous") | ||
| Expand All | @@ -3996,7 +4023,7 @@ def rotate_from_to(cls, r1, r2): | |
| else: | ||
| q = cls(1, [0, 0, 0]) # = 1, no rotation | ||
| else: | ||
| q = cls(math.cos(th), k*math.sin(th)/nk) | ||
| q = cls(np.cos(th), k*np.sin(th)/nk) | ||
| return q | ||
|
|
||
| @classmethod | ||
| Expand All | @@ -4021,10 +4048,11 @@ def as_cardan_angles(self): | |
| """ | ||
| The inverse of `from_cardan_angles()`. | ||
| Note that the angles returned are in radians, not degrees. | ||
| The angles are not sensitive to the quaternion's norm(). | ||
| """ | ||
| qw = self.scalar | ||
| qx, qy, qz = self.vector[..., :] | ||
|
Comment thread
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIt won't let me add this comment to the unmodified line below, but in playing with this PR I do infrequently run into a domain error on arcsin due to floating point errors. A little hard to reproduce reliably, but it does pop up. Clipping the inside value to [-1, 1] should fix this. /mnt/c/Users/Scott/Documents/Documents/Coding/matplotlib/lib/mpl_toolkits/mplot3d/axes3d.py:4028: RuntimeWarning: invalid value encountered in arcsin elev = np.arcsin( 2*( qw*qy+qz*qx)/(qw*qw+qx*qx+qy*qy+qz*qz)) # noqa E201 posx and posy should be finite values
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
I'm just so curious about the values of qw, qx, qy, and qz that cause this. I can understand the posx and posy should be finite values complaint, as a consequence of the arcsin argument getting out of range. But the invalid value encountered in arcsin is still mysterious... try:
elev = np.arcsin( 2*( qw*qy+qz*qx)/(qw*qw+qx*qx+qy*qy+qz*qz)) # noqa E201
except:
print(repr(qw), repr(qx), repr(qy), repr(qz))
print(repr(qw*qy), repr(qz*qx), repr(qw*qw), repr(qx*qx), repr(qy*qy), repr(qz*qz) )
print(repr( 2*( qw*qy+qz*qx) ), repr(qw*qw+qx*qx+qy*qy+qz*qz) )
print(repr( 2*( qw*qy+qz*qx)/(qw*qw+qx*qx+qy*qy+qz*qz) ))
(I thought the error merits an attempt at diagnosing; not that I'm opposed to the np.clip(), I just thought it would be good to understand what is going on, how this comes about.)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI just spent a few minutes trying to reproduce but couldn't - it's a tricky edge case to trigger. When I ran across it before, the value inside arcsin was only 1e-16 (or thereabouts) larger than 1, so it's just a result of numerical round-off. My guess would be a conditioning issue, where one of the values is very small relative to the others and gets rounded off to 0 in the denominator, but is big enough to still impact the numerator. I'll leave the debug lines in and let you know if it happens again, but am not concerned - this stuff end up happening fairly regularly across the codebase.
Sorry, something went wrong.
MischaMegens2 reacted with thumbs up emoji
MischaMegens2 reacted with heart emoji
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI think the np.clip got missed in your latest commit, possible to add that quickly before merge?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Oh sorry, of course; I put the np.clip() back in.
Sorry, something went wrong.
scottshambaugh reacted with thumbs up emoji
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThanks! Need to move the close paren to fix the test failure but the MR is looking good.
Sorry, something went wrong.
MischaMegens2 reacted with confused emoji
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
You had mentioned before that there were floating point round-off errors on the macos github CI runners (#28823 (comment)); I think that was the original motivation to put the np.clip() in in the first place. Any chance we can trigger the edge case there once again? (Not that I'm against the np.clip(), but I'm still surprised by the occurrence of the quotient >1. I thought the IEEE 754 standard for floating-point arithmetic requires that multiplication and addition should be correctly rounded, so I thought we would be in the clear... unless macos would not conform to IEEE 754, which would be also quite remarkable... Or I don't quite understand all of the correctly, and I should go read some more Kahan)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIf I happen to catch it again I'm happy to add it as a test! I think I lost the specific test case in the other MR that was erroring, and it was dependent on the specific math operations being performed so it likely wouldn't translate to this new code. To be clear, I don't think it's a macos-specific problem, the issue is more that there isn't perfect determinism across different platforms / python versions / etc (even with perfect determinism within a configuration). Everything is being rounded "correctly", it's just that if we're right on the edge of floating point tolerance, complier/processor/implementation/optimization differences can have different results, and values infinitesimally on the wrong side of 1 will stack up unfavorably with further operations. For a simple concrete example, some systems in calculating (a + b)*c might distribute that multiplication to a*c + b*c, and those can result in different rounding of the results. Especially if for example a >> b and so b doesn't make it into the mantissa of a + b (addition and subtraction are not associative in floating point math, which is pretty unintuitive IMO).
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
I'm just trying to wrap my head around what the optimization difference could have been that would lead to the observed result. '(a+b)c' -> 'ac + b*c' does not quite fit the bill... My hope is that an actual example would shed light on it...
Sorry, something went wrong.
All reactions
|
||
| azim = np.arctan2(2*(-qw*qz+qx*qy), qw*qw+qx*qx-qy*qy-qz*qz) | ||
| elev = np.arcsin( 2*( qw*qy+qz*qx)/(qw*qw+qx*qx+qy*qy+qz*qz)) # noqa E201 | ||
| roll = np.arctan2(2*( qw*qx-qy*qz), qw*qw-qx*qx-qy*qy+qz*qz) # noqa E201 | ||
| elev = np.arcsin(np.clip(2*(qw*qy+qz*qx)/(qw*qw+qx*qx+qy*qy+qz*qz), -1, 1)) | ||
| roll = np.arctan2(2*(qw*qx-qy*qz), qw*qw-qx*qx-qy*qy+qz*qz) | ||
| return elev, azim, roll | ||
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.