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

compilation error in test file #372

Closed
doudou opened this issue Feb 11, 2019 · 4 comments · Fixed by #373
Closed

compilation error in test file #372

doudou opened this issue Feb 11, 2019 · 4 comments · Fixed by #373

Comments

@doudou
Copy link

doudou commented Feb 11, 2019

fcl/test/narrowphase/detail/convexity_based_algorithm/test_gjk_libccd-inl_gjk_doSimplex2.cpp:181:67: note:   deduced conflicting types for parameter ‘_Tp’ (‘double’ and ‘float’)
     const ccd_real_t eps = std::max({1., p_OA.norm(), p_OB.norm()}) * kEps;

Changing the 1. to 1.0f does fix the error, but I'm guessing that the double/float error might be a configuration thing (maybe ?).

@sherm1
Copy link
Member

sherm1 commented Feb 12, 2019

Whoops! Should be T(1) (where T is the template argument) to avoid committing to float or double prematurely (assuming we're still supporting float precision).

This suggests that CI doesn't include float tests?

@SeanCurtis-TRI
Copy link
Contributor

Actually, in this case, it should be ccd_real_t(1). This test isn't expressed in terms of T, but in terms of ccd_real_t. And, clearly, this test is running against a float-based libccd. I'll patch the test.

@SeanCurtis-TRI
Copy link
Contributor

SeanCurtis-TRI commented Feb 12, 2019

The PR for this has just been posted. I've confirmed the reported compilation error and that this fixes it (on my machine). @doudou, if you'd like to confirm as well, that'd be appreciated. Otherwise, we'll just go straight through PR review and merge it.

@doudou
Copy link
Author

doudou commented Feb 12, 2019

@SeanCurtis-TRI yep, #373 does the trick

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 a pull request may close this issue.

3 participants