-
Notifications
You must be signed in to change notification settings - Fork 23
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
Unit-Safe Implementation of std::hypot
#384
Unit-Safe Implementation of std::hypot
#384
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this contribution! I'll re-review once you get the CLA signed, and remove the QuantityPoint
overloads.
Co-authored-by: Chip Hogg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM with just one more small change needed. (I spotted the change manually, and then in running the jobs, it looks like it is in fact causing failures.)
You'll also need to sign the CLA. Let me know if you're having any difficulties with that, and I'll try to help troubleshoot!
Co-authored-by: Chip Hogg <[email protected]>
Thanks for the speedy review. Regarding the CLA, I signed the corporate version on Friday. It should be under my name with the company name of "Swarmbotics AI". It does not look like CLAassistant is finding it. Do I need to sign the individual one as well? |
I have reached out internally to the folks who manage our CLA assistant. I hope we can get this cleared up pretty soon. |
Hi @justin-kottinger, thanks for your contribution here! For corporate CLA's, our options are to manually allowlist either:
Does "Swarmbotics AI" have a public GitHub organization that we can list here? That would be preferable over maintaining a list of usernames. Thanks! [EDIT] I've found your fork here: https://github.com/SwarmboticsAI/au (which I missed earlier), so I'll use that 👍 The next step is to make yourself a public member of that org. Right now I'm seeing No public members, so the CLA Assistant is not able to verify your membership of this org. Can you please update your org visibility please? |
Also, note that while looking into this internally, we found that you're the first contributor to use the corporate CLA. 😄 So thanks for helping us work out the kinks, and I hope it explains why there are any kinks! |
Summary
Implements a unit-safe version of
std::hypot
within the AU library. This implementation ensures dimensional consistency when calculating the hypotenuse/Euclidean norm while preserving the underlying units.Implementation Details
std::hypot
Related to #383