-
Notifications
You must be signed in to change notification settings - Fork 183
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
Throw on box change #2031
Throw on box change #2031
Conversation
Some test cases might still contain a check for the constraint feature. E.g. constraint_shape_based.py |
Ready for review. |
src/core/constraints/Constraints.hpp
Outdated
void on_boxl_change() const { | ||
if (not this->empty()) { | ||
throw std::runtime_error("The box size can not be changed because there " | ||
"are active constraints. " + |
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.
Can you please change the order of the strings, we get a nicer error message then.
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.
LGTM besides comment
Codecov Report
@@ Coverage Diff @@
## python #2031 +/- ##
========================================
Coverage ? 63%
========================================
Files ? 443
Lines ? 28239
Branches ? 0
========================================
Hits ? 17791
Misses ? 10448
Partials ? 0
Continue to review full report at Codecov.
|
Description of changes: - Resizing the box now throws a runtime error if there are constraints present, since constraint preconditions might no longer be fulfilled (e.g. a wall constraint might end up outside the box boundaries when the box shrinks) - This feature was originally introduced in ESPResSo 4.0.0 (#2031), but never actually worked because the PR accidentally removed the event function call during merge conflict resolution
Description of changes: - Resizing the box now throws a runtime error if there are constraints present, since constraint preconditions might no longer be fulfilled (e.g. a wall constraint might end up outside the box boundaries when the box shrinks) - This feature was originally introduced in ESPResSo 4.0.0 (espressomd#2031), but never actually worked because the PR accidentally removed the event function call during merge conflict resolution
Preliminary throw an error if the box size is changed with active constraints,
this does not work properly with all shapes and can have some non-intuiitive
side effects. Also removed CONSTRAINT feature and moved constraints out
of the short range loop.