-
-
Notifications
You must be signed in to change notification settings - Fork 18
Add property-based tests for cross-products #70
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
Conversation
|
@astronouth7303 The numerical instability revealed by the tests has been recently studied by W. Kahan (yes, the same one who had useful information about the instabilities in angle computations). |
AstraLuma
left a comment
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.
Is there a way to test around the instability? If this is an on-going area of study, I think we should just accept it and work with/around it.
|
PS: #65 was merged without this, so it should be re-targetted against master |
|
There is a failure of the linearity test, as well. I assume this is the instability you've referred to before? |
1867143 to
dc35a99
Compare
dc35a99 to
38437b3
Compare
|
@astronouth7303 @pathunstrom I was poking at making the cross-product more numerically stable, when I realised that there's nothing that uses it anymore since #68 was merged. I kinda feel like removing the feature outright might be sensible, since providing a well-behaved cross-product is pretty hard. What do you think? |
|
This seems reasonable, since this only exists because you needed it for some algorithm. Is there something external that you'd want it for? Intersections? Sidedness? |
|
It can be used for sidedness, yes. If someone decides they need it, it would be pretty easy to bring it back, but AFAIK it never was in any release, so it should be fine to drop it? And yes, I initially added it for implementing |
|
PS: Just pushed some local changes I had, for the record, but if you confirm we can remove the feature, I would rather do that. |
Cf ppb#70 for the rationale. TL;DR: - hard to implement in a stable and predictable way; - initially added to implement `Vector2.angle`, which doesn't use it anymore; - was never included in a release of ppb-vector.
|
@astronouth7303 Mmmh, cross-product is also nice to check if vectors are aligned. |
|
Pretty sure dot product can be used for the same purpose? At least all the math for game dev tutorials I've read use the dot product. |
|
@pathunstrom The dot product is a bit more annoying to use for that purpose, IMO (you have to normalize & such). OTOH, I just realised this makes no sense, we have a perfectly fine |
|
Replying to @astronouth7303's comments on #80 (I think it would be better to keep the conversation here) :
Nothing in the library use cross-product, and I can't think of anything I would want in a game that benefits from it; for general geometry, cross-product is pretty convenient for computing the area of triangles or parallelograms between 2 vectors (area_plg = 2*area_triangle = abs(a ^ b)).
Yes, the instability occurs when there are |
|
That's a pretty significant instability (10% will probably have noticeable effects in a game) for normal vectors. I think that qualifies cross product as a foot-gun and it should probably be removed on that premise. |
|
@pathunstrom your vote? |
|
I'm all for keeping foot guns out of the ppb name space. |
|
❤️ |
PR targetting the branch for #65