Opened 15 years ago
Closed 15 years ago
#2859 closed defect (fixed)
[with patch, positive review] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow
Reported by: | jason | Owned by: | was |
---|---|---|---|
Priority: | blocker | Milestone: | sage-3.0 |
Component: | graphics | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Try the following:
plot(vector((0,0,-1)))
The resulting vector points up, but should point down.
Attachments (1)
Change History (12)
comment:1 Changed 15 years ago by
Summary: | plotting the vector (0,0,-1) really plots (0,0,1) → arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow |
---|
comment:2 Changed 15 years ago by
All right, this was because we were taking the cross product with (0,0,1) and using that to decide whether or not to rotate the vector, ignoring the case that we point in the opposite direction (but still have a cross product of zero).
Patch will be attached. I'm marking this a blocker since it gives a *wrong* result and William said that wrong results should be marked as blockers.
comment:3 Changed 15 years ago by
Priority: | major → blocker |
---|
comment:4 Changed 15 years ago by
Summary: | arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow → [with patch, needs review] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow |
---|
comment:5 Changed 15 years ago by
comment:6 Changed 15 years ago by
It doesn't relate.
In this ticket, the arrow is actually wrong (the assumption in the code is that "parallel to (0,0,1)" == "pointing in the same direction as (0,0,1)", which is just wrong.
The issue in #1777 is a cosmetic issue related to the menus in JMOL.
comment:7 Changed 15 years ago by
Summary: | [with patch, needs review] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow → [with patch, positive review pending] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow |
---|
The patch works as advertised and the code looks good to me. Positive review for the code, and I'll give it full positive review once you sort out this minor complaint about the doctest: instead of "A downward pointing arrow should have a transformation scaling the points to their negatives" do you mean to say something like "a downward-pointing arrow corresponds to a transformation which reverses the z-coordinates of points"?
Mostly I'm confused because when you give me a vector in R3 and ask me what linear transformation of R3 to itself that it's describing, I naturally think of reflection across the perpendicular plane. The vector (0,0,-1) doesn't say "take (x,y,z) to (-x,-y,-z)" to me...but maybe this is just me, and perhaps it's more an issue with get_transformation
than with arrow3d
.
At any rate, if you're a bit more clear in that description, you'll be good to go.
comment:8 Changed 15 years ago by
Arrows are always constructed pointing up in the z direction from the origin, and then rotated/translated into place. This works for every arrow direction except the -z direction. I take care of the issue by testing to see if the arrow should point in the -z direction, and if it should, just scaling the constructed arrow by -1 (i.e., every point is sent to its negative). The scaled arrow then points downwards. The doctest just tests that the scale of -1 is applied to the arrow. I'm not sure how to make it clearer.
Changed 15 years ago by
Attachment: | trac-2859-arrow3d.patch added |
---|
comment:9 Changed 15 years ago by
I put up a new patch with the above explanation above the doctest.
comment:10 Changed 15 years ago by
Summary: | [with patch, positive review pending] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow → [with patch, positive review] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow |
---|
Looks good to me. I like the added explanation. Positive review.
Cheers,
Michael
comment:11 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Merged in Sage 3.0.alpha3
The problem is in arrow3d: