Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Round, Rounded, Correct, Distance(x, y, z, w) and operator< addition to Vector 4 #146

Merged
merged 5 commits into from
Aug 7, 2020

Conversation

luccosta
Copy link
Contributor

Resolves #71 .

This implements the functions Round(), Rounded(), Correct(), Distance(T x, T y, T z, T w) and operator overload operator<(Vector4) in Vector4 API.

It enables:

vec4.Round();
Vector4 roundedVector = vec4.Rounded();
vec4.Correct();
vec4.Distance(1, 2, 3, 4);
if (vec4_a < vec4_b) {
  ...
}

@luccosta luccosta requested a review from scpeters as a code owner July 31, 2020 23:01
@luccosta
Copy link
Contributor Author

luccosta commented Jul 31, 2020

I tried to test the Correct function generating an NaN first with 0/0 but this generate an Exception and them with math::sqrt(-1) that works fine, but generate the following message in code checker:

(warning) Passing value -1 to sqrt() leads to implementation-defined result.

Should I change/remove the test?

@chapulina
Copy link
Contributor

test the Correct function generating an NaN

While working on #139 I noticed that the code checker would complain about 0.0 / 0.0 but not 1.0 / 0.0, maybe this is an option?

Also another option may be to use std::nan().

@luccosta
Copy link
Contributor Author

luccosta commented Aug 1, 2020

but not 1.0 / 0.0, maybe this is an option?

Is a good option, I will check the result.

Also another option may be to use std::nan().

Looks like a more consistent test, I will let with this.

Signed-off-by: Lucas Fernando <[email protected]>
@luccosta
Copy link
Contributor Author

luccosta commented Aug 1, 2020

I will check the result.

With 1/0 I get Floating-point exception running the test as with 0/0.

@mjcarroll
Copy link
Contributor

I don't think that you'll be able to get around the divide-by-zero issue either triggering the code check, compiler warning, or runtime error (since it's a signaling nan).

I agree that using std::nan here is the best approach, since that generates a quiet nan and won't trigger any floating point exceptions.

@luccosta
Copy link
Contributor Author

luccosta commented Aug 3, 2020

I agree that using std::nan here is the best approach

Great, the current PR is using just std::nan.

@mjcarroll mjcarroll merged commit 1a7ae9f into gazebosim:ign-math6 Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants