commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <>
Subject [jira] [Commented] (GEOMETRY-17) Euclidean Vector Method Follow-Up
Date Sat, 22 Sep 2018 11:10:00 GMT


Gilles commented on GEOMETRY-17:

Side note: It becomes uneasy to follow through all the changes; we should strive to make patches
as small as possible.

bq. renamed the getRealNonZeroNorm() method to getCheckedNorm() and made it private

Good; that gives us some time to think more about it.

However you did not comment on my {{withCheckedNorm()}} (untested) suggestion.

bq. MultiDimensionalEuclideanVector

The suggestion looks fine.

bq. just propagating NaNs/infs but in the case of angle(), we might actually be producing

Well, to propagate something, it must be produced somewhere. ;)
If {{angle()}} produces NaN, it is because it was called inappropriately (if the caller carelessly
uses the return value). I think that propagating, or not, is a question of performance; but
we are be safer with on-the-spot error check.

bq. I did not make the UnitVector getNorm() and getNormSq() methods return 1.0 \[...\] not
sure how doing so affects the overall floating point accuracy

I don't see how the constant 1 can be less accurate than a variable holding a value that is
supposed to be 1!

bq. some unit tests in commons-geometry-spherical started failing

(!) This is quite unexpected and should be tracked.

> Euclidean Vector Method Follow-Up
> ---------------------------------
>                 Key: GEOMETRY-17
>                 URL:
>             Project: Apache Commons Geometry
>          Issue Type: Improvement
>            Reporter: Matt Juntunen
>            Priority: Major
> This is a follow-up issue to GEOMETRY-9. The following tasks should be completed:
>  # Vector2D - needs an orthogonal() method like Vector3D
>  # Vector#getMagnitude() should be removed. I originally added this as part of GEOMETRY-9
as an alias for getNorm(), but after thinking about it more and working with it, I believe
it's more confusing than useful to have multiple names in the code base for the same idea.
>  # Vector#withMagnitude() should be renamed to Vector#withNorm() for the same reason
as above.
>  # Vector#getRealNonZeroNorm() - This is currently a private method in the Vector implementation
classes but I believe it is useful enough to be made public. The idea is that this would return
the vector norm but throw an IllegalNormException if the norm is zero, NaN, or infinite. I've
already come across some places in other classes (such as Rotation) where I want to use this.
> Pull request:

This message was sent by Atlassian JIRA

View raw message