This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 601 - Quaternion and AngleAxis should check their input
Summary: Quaternion and AngleAxis should check their input
Status: DECISIONNEEDED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Geometry (show other bugs)
Version: 3.2
Hardware: All All
: Low API Change
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on: 560
Blocks: 3.x
  Show dependency treegraph
 
Reported: 2013-05-17 10:29 UTC by Gael Guennebaud
Modified: 2019-12-04 12:19 UTC (History)
4 users (show)



Attachments
Proposed patch (10.39 KB, application/octet-stream)
2013-05-17 10:29 UTC, Gael Guennebaud
no flags Details

Description Gael Guennebaud 2013-05-17 10:29:41 UTC
Created attachment 337 [details]
Proposed patch

As discussed on the mailing list, some of the ctors/members of AngleAxis and Quaternion assume either unit vectors of matrices. The attached patch does exactly that with a very high tolerance to reduce the risk of false negatives:

sqrt(NumTraits<Scalar>::dummy_precision())

Such a high threshold was needed to be compatible with fast normalization routines that are very inaccurate.

There was discussion on whether this threshold should be configurable by the user. Sounds overkill to me.
Comment 1 Christoph Hertzberg 2013-05-17 11:18:22 UTC
As said on the mailing list, my only concern with that patch is that it breaks functionality for those few users who use quaternions in a general algebraic way.
I made this bug depend on bug 560 and block 3.2 (although 560 does not need to block 3.2, but the API decision should be made).

I agree on the non-configurability.
Also the proposed threshold seems fine to me. Users working with double and requiring less precision should switch to single precision anyways.
Comment 2 Gael Guennebaud 2013-06-10 10:51:21 UTC
since this concerns API decision, let's defer it for 3.3 as I'd like to release 3.2 asap.
Comment 3 Gael Guennebaud 2015-03-06 18:47:50 UTC
Related issue on the forum: https://forum.kde.org/viewtopic.php?f=74&t=125239
Comment 4 Gael Guennebaud 2016-01-30 21:43:19 UTC
Still no consensus -> 3.4
Comment 5 Nobody 2019-12-04 12:19:29 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/601.

Note You need to log in before you can comment on or make changes to this bug.