mathnet / mathnet-spatial

Math.NET Spatial
http://spatial.mathdotnet.com
MIT License
376 stars 132 forks source link

Cross product error checks are different depending on whether you pass it a UnitVector3D or a Vector3D #196

Closed DaveInCaz closed 1 year ago

DaveInCaz commented 2 years ago

Cross product behaves differently depending on whether you pass it a UnitVector3D or a Vector3D, even if they are numerically identical.

Example (from the VS immediate window):

UnitVector3D.ZAxis.CrossProduct(UnitVector3D.ZAxis)

'UnitVector3D.ZAxis.CrossProduct(UnitVector3D.ZAxis)' threw an exception of type 'System.InvalidOperationException'
    Data: {System.Collections.ListDictionaryInternal}
    HResult: -2146233079
    HelpLink: null
    InnerException: null
    Message: "The Euclidean norm of x, y, z is less than float.Epsilon"
    Source: "MathNet.Spatial"
    StackTrace: "   at MathNet.Spatial.Euclidean.UnitVector3D.Create(Double x, Double y, Double z, Double tolerance)\r\n   at MathNet.Spatial.Euclidean.UnitVector3D.CrossProduct(UnitVector3D other)"
    TargetSite: {MathNet.Spatial.Euclidean.UnitVector3D Create(Double, Double, Double, Double)}

but in contrast:

UnitVector3D.ZAxis.CrossProduct(1 * UnitVector3D.ZAxis)

{(0, 0, 0)}
    Length: 0
    Orthogonal: 'UnitVector3D.ZAxis.CrossProduct(1 * UnitVector3D.ZAxis).Orthogonal' threw an exception of type 'System.InvalidOperationException'
    X: 0
    Y: 0
    Z: 0

The reason for this seems to be that CrossProduct(UnitVector3D) internally calls Create(...) but CrossProduct(Vector3D) does not.

Unless there is a reason for this discrepancy (?) it seems unintentional. I'd be happy to submit a PR so that both would use Create if that can be confirmed.

Thanks

jkalias commented 1 year ago

Hi @DaveInCaz ,

This is definitely not correct. I plan a bigger rewrite based on #200 but until then, please feel free to send a PR.

DaveInCaz commented 1 year ago

Thank you for addressing this. Sorry I hadn't had a chance to reply earlier.

jkalias commented 1 year ago

Sure, no worries