Conversation
Merge pull request michellab#78 from Roy-Haolin-Du/main
Added nmoves as configurable and changed ncycles to property, added t…
|
Fixes #79 - Adds ncycles support to prevent memory errors. |
| ncycles_int = round(float(ncycles)) | ||
|
|
||
| if ncycles_int < 1: | ||
| raise ValueError( |
There was a problem hiding this comment.
I know this was done before for timestep and runtime, but I think it might be better to validate all of this stuff in a validator, rather than in the property. Then, an error will be raised on instantiation/ assignment (we've set it to validate on assignment) rather than later when the user tries to call the property. The property can then just compute ncycles.
| """ | ||
| Make sure runtime is a multiple of timestep | ||
| Calculate number of cycles from runtime, nmoves and timestep. | ||
| Formula: runtime = nmoves × ncycles × timestep |
There was a problem hiding this comment.
I might not be remembering how these settings work, but my main comment is: do we know if this will work with adaptive computations where the assigned times may be pretty short? 25000 * 4 fs = 100 ps which is a fairly large smallest runtime. Have you tried this with the adaptive runs?
A simple way to make this more flexible would be to replace nmoves with max_nmoves. This would modify the behaviour of the previous nmoves property so that the behaviour stays as it was unless runtime > max_nmoves * timestep, in which case the nmoves gets e.g. halved and ncycles doubled or something similar. We'll need to be careful to make sure you get a valid nmoves and ncycles in this case (easiest way might be to enforce that runtime must be a multiple of 2 * timestep).
|
Thanks Roy! Mainly looks good but have left a couple of comments. |
2 participants
Description
Added nmoves as a configurable field and changed ncycles to a computed property
Todos
Added nmoves as a configurable field and changed ncycles to a computed property in
engine_config.pyto prevent memory overflow from single-cycle runtimes.Status