-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add error when ellipsoid axis is zero #1785
Conversation
I think a check against 0 is correct here. |
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.
The pull request is straightforward, but we should raise a similar error message for FacetedEllipsoid
.
pre-commit.ci autofix |
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!
Description
A check was added to verify values are within the expected range.
I set the minimum allowed value toI am open to suggestions about the correct value to assign.ELLIPSOID_OVERLAP_PRECISION
(1e-6), which may or may not be the correct choice. We could do FLT_EPSILON from as well, but that may not be consistent ifHOOMD_SHORTREAL_SIZE==64
.Errors occur then any semimajor axis is less than
0.0f
.Motivation and context
Overlap checks do not work for ellipsoids with any semimajor axis less than zero. This error (on the c++ level) prevents users from inadvertently encountering this issue.
Resolves #1762
How has this been tested?
A new test has been added that attempts to set invalid parameters. This tests values at zero, slightly above zero, and well below it.
Change log
Checklist:
sphinx-doc/credits.rst
) in the pull request source branch.