tune_skin: add adjust_max_skin feature. #3011
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jngrad
reviewed
Jul 25, 2019
|
the "unhandled error" indicates that at the end of the tune_skin function in python utils.handle_errors() needs to be called. That would then raise an exception Can you please add a test based on the script in the comment. Please set adjust_max_skin=True in maintainer/benchmarks/p3m.py. That may solve #2924. |
|
Now, with proper error handling |
Codecov Report
@@ Coverage Diff @@
## python #3011 +/- ##
======================================
- Coverage 82% 82% -1%
======================================
Files 526 526
Lines 26754 26758 +4
======================================
- Hits 22041 22025 -16
- Misses 4713 4733 +20
Continue to review full report at Codecov.
|
jngrad
reviewed
Jul 25, 2019
| @@ -77,6 +77,7 @@ foreach(TEST_COMBINATION lb.cpu-p3m.cpu-lj;lb.gpu-p3m.cpu-lj;ek.gpu) | |||
| endforeach(TEST_BINARY) | |||
| endforeach(TEST_COMBINATION) | |||
| python_test(FILE cellsystem.py MAX_NUM_PROC 4) | |||
| python_test(FILE tune_skin.py MAX_NUM_PROC 1) | |||
There was a problem hiding this comment.
@RudolfWeeber how many threads should we use here?
Merge conflict |
|
bors r=RudolfWeeber |
bors bot
added a commit
that referenced
this pull request
Jul 26, 2019
3011: tune_skin: add adjust_max_skin feature. r=RudolfWeeber a=pkreissl Fixes #2961 Description of changes: - Add `adjust_max_skin` parameter to `tune_skin()` method, which adds the option to automatically adjust `max_skin` if the passed value is larger than permitted (0.5 * local box length - short range cutoff) - Rename core variables to match interface. As a test you can run the following script adjusted from @RudolfWeeber s minimal script in issue #2961 ```python import espressomd s = espressomd.System(box_l=[1,1,1]) s.time_step = 0.01 s.non_bonded_inter[0,0].lennard_jones.set_params(epsilon=1,sigma=0.2,cutoff=0.3,shift="auto") print("=> Tune with adjustment:") s.cell_system.tune_skin(min_skin=0.1,max_skin=0.6,tol=0.05, int_steps=3, adjust_max_skin=True) print(s.cell_system.skin) print("=> and fail without:") s.cell_system.tune_skin(min_skin=0.1,max_skin=0.6,tol=0.05, int_steps=3) ``` resulting in the following output: ``` $ ./pypresso tune_skin.py => Tune with adjustment: 0.17500000000000002 => and fail without: There were unhandled errors. ERROR: number of cells 1 is smaller than minimum 8 (interaction range too large or min_num_cells too large) ``` Co-authored-by: Patrick Kreissl <patrick.kreissl@pa-le.de> Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
Timed out |
|
bors retry |
bors bot
added a commit
that referenced
this pull request
Jul 26, 2019
3011: tune_skin: add adjust_max_skin feature. r=RudolfWeeber a=pkreissl Fixes #2961 Description of changes: - Add `adjust_max_skin` parameter to `tune_skin()` method, which adds the option to automatically adjust `max_skin` if the passed value is larger than permitted (0.5 * local box length - short range cutoff) - Rename core variables to match interface. As a test you can run the following script adjusted from @RudolfWeeber s minimal script in issue #2961 ```python import espressomd s = espressomd.System(box_l=[1,1,1]) s.time_step = 0.01 s.non_bonded_inter[0,0].lennard_jones.set_params(epsilon=1,sigma=0.2,cutoff=0.3,shift="auto") print("=> Tune with adjustment:") s.cell_system.tune_skin(min_skin=0.1,max_skin=0.6,tol=0.05, int_steps=3, adjust_max_skin=True) print(s.cell_system.skin) print("=> and fail without:") s.cell_system.tune_skin(min_skin=0.1,max_skin=0.6,tol=0.05, int_steps=3) ``` resulting in the following output: ``` $ ./pypresso tune_skin.py => Tune with adjustment: 0.17500000000000002 => and fail without: There were unhandled errors. ERROR: number of cells 1 is smaller than minimum 8 (interaction range too large or min_num_cells too large) ``` Co-authored-by: Patrick Kreissl <patrick.kreissl@pa-le.de> Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2961
Description of changes:
adjust_max_skinparameter totune_skin()method, which adds the option to automatically adjustmax_skinif the passed value is larger than permitted (0.5 * local box length - short range cutoff)As a test you can run the following script adjusted from @RudolfWeeber s minimal script in issue #2961
resulting in the following output: