[
https://issues.apache.org/jira/browse/GEOMETRY-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17314489#comment-17314489
]
Alex Herbert commented on GEOMETRY-119:
---------------------------------------
{quote}0x1.0p1024 is out of range
{quote}
Of course :)
I think not handling overflow but handing tiny valued vectors is a half
solution. I would prefer to detect and do the scaling for either case. If using
scaling then the only solution that will not produce a result is an input with
non-finite coordinates. Anything with finite coordinates should be possible to
normalise.
With regard to the return of the Optional, can you update the JMH performance
test to see if creation of the extra object offsets the previously seen
performance gain. I would also add a version that does not call Vectors3D.norm
and uses sqrt(x^2+y^2+z^2). It may be the large part of the runtime is actually
spent in that method which has a lot of conditional statements. You can shift
those conditionals to the scale detection in tryCreateNormalized.
This may be the most performant version:
{code:java}
private static Unit tryCreateNormalized(final double x, final double y, final
double z,
final boolean throwOnFailure) {
final double max = Math.max(Math.max(Math.abs(x), Math.abs(y)),
Math.abs(z));
if (max > 0x1.0p500) {
// Scale down
x *= 0x1.0p-600;
y *= 0x1.0p-600;
z *= 0x1.0p-600;
} else if (max < 0x1.0p-500) {
// Scale up
x *= 0x1.0p600;
y *= 0x1.0p600;
z *= 0x1.0p600;
}
final double normInv = 1.0 / Math.sqrt(x * x + y * y + z * z);
if (Double.isFinite(normInv)) {
return new Unit(x * normInv, y * normInv, z * normInv);
} else if (throwOnFailure) {
// Input must have infinite or NaN coords
throw Vectors.illegalNorm(norm);
}
return null;
}
{code}
Since we have used scaling this drops the Vectors.isRealNonZero(normInv) check.
It would not be possible to create a zero value as large input has been reduced
in size so that the inverse will be above zero, e.g. this passes:
{code:java}
for (double x : new double[]{Double.MIN_VALUE, 0x1.0p-500, 0x1.0p500,
Double.MAX_VALUE}) {
if (x > 0x1.0p+500) {
x *= 0x1.0p-600;
} else if (x < 0x1.0p-500) {
x *= 0x1.0p+600;
}
assert 1.0 / Math.sqrt(x * x * 3) > 0.0;
}
{code}
> Vector normalizeOrDefault() method
> ----------------------------------
>
> Key: GEOMETRY-119
> URL: https://issues.apache.org/jira/browse/GEOMETRY-119
> Project: Apache Commons Geometry
> Issue Type: Improvement
> Reporter: Matt Juntunen
> Priority: Major
>
> A frequent use case when working with vectors, especially vectors coming from
> external data, is attempting to normalize the vector, and if this is not
> possible, to use an alternative value. For example, the
> {{QuaternionRotation}} code
> [here|https://github.com/apache/commons-geometry/blob/master/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java#L86]
> does exactly this; it attempts to normalize a vector and failing that (ie,
> if the vector is exactly zero), it returns a substitute value. The
> {{QuaternionRotation}} class is able to take advantage of our internal
> {{Vectors.tryNormalize()}} but callers outside of the library are not able to
> do so and so are left with 2 choices:
> 1. wrap the {{normalize()}} call in a try-catch and handle the exception
> thrown on illegal norm values, or
> 2. compute and test the norm prior to calling {{normalize()}} to ensure that
> the call won't fail, resulting in 2 computations of the norm.
> Neither of these options are very good.
> I propose adding a new method to the Euclidean Vector classes to handle this
> situation: {{normalizeOrDefault()}}. The method would accept a default value
> (possibly null) to return if the vector cannot be normalized. The normal
> would then only need to be computed once and an exception would not need to
> be thrown in case of failure. The behavior of the current {{normalize}}
> method would be the same.
> Examples:
> {code:java}
> // get some kind of normal, preferably vec but +z will also do
> Vector3D.Unit norm = vec.normalizeOrDefault(Vector3D.Unit.PLUS_Z);
> {code}
> {code:java}
> // throw a very use-case specific error message
> Vector3D norm = vec.normalizeOrDefault(null);
> if (norm == null) {
> throw new Exception("Invalid triangle at index " + i + ": cannot compute
> normal.");
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
