[ 
https://issues.apache.org/jira/browse/GEOMETRY-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17116085#comment-17116085
 ] 

Matt Juntunen commented on GEOMETRY-50:
---------------------------------------

Revisiting this...

I took another look at the performance numbers here, checking JDK 8 and JDK 11. 
Using {{SafeNorm}} is ~20 times faster on JDK 8 (vs {{Math.hypot}}) and ~2 
times faster on JDK 11 for 2D norm calculation. For 3D it is ~10 slower than 
the current unchecked implementation (raw floating point operations) on both 
JDKs. So, it's something of a mixed bag in terms of performance. 

I also noticed an interesting thing in the Vector2D unit tests while making 
this change. The test 
[testOrthogonal_fullCircle|https://github.com/apache/commons-geometry/blob/master/commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/twod/Vector2DTest.java#L469]
 began failing on the dot product comparison on line 479 after the switch to 
{{SafeNorm}}. This test creates vectors of length pi pointing in directions all 
around the circle, computes the orthogonal vectors from each, and then checks 
that the orthogonal vector has unit length and a dot product of zero with the 
original vector. The original test epsilon for the dot product comparison was 
{{Math.ulp(1d) = 2.220446049250313E-16}}. After the switch from {{Math.hypot}} 
to {{SafeNorm}}, one of the produced dot products changed from 
{{-5.8839292547567834E-18}} to {{-2.2617374526731045E-16}}, causing the test to 
fail. Changing the test epsilon to {{Math.ulp(Math.PI) = 
4.440892098500626E-16}} (the length of the input vector) makes the test pass. 
Is this result to be expected here? I would hope that {{Math.hypot}} and 
{{SafeNorm}} would have returned the same numbers.

> Overflow in Vector norm and distance
> ------------------------------------
>
>                 Key: GEOMETRY-50
>                 URL: https://issues.apache.org/jira/browse/GEOMETRY-50
>             Project: Apache Commons Geometry
>          Issue Type: Bug
>            Reporter: Baljit Singh
>            Priority: Major
>              Labels: beta1
>
> In Euclidean Vector classes (Vector2D, Vector3D), norm() and distance() rely 
> on Math.sqrt(), which can overflow if the components of the vectors are 
> large. Instead, they should rely on SafeNorm.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to