Add final restart files, and fixed heating file stems #116
Conversation
|
One thought maybe instead of |
|
can be but be careful cause STEP and FINAL_STEP may be the same and you do not want to write twice... |
true it could cause overwriting to happen, I'll leave as final for now |
|
you can have a check to write it only if it does not exist already |
harveydevereux
force-pushed
the
final_restart
branch
from
c9c6431
to
8363071
Compare
|
Tests now pass, with some modifications since pressure and temperature have been permuted, and also since restart now adds temperature. Will add tests for the heating and final restart files next |
At the moment these changes seems to output the "final" structure at every temperature in the ramp, not just at the end of heating? Was this intended? |
|
What's the benefit in all of these extra files, in addition to the periodic restart files (and trajectory)? I see the benefit in storing a final structure at the end of whatever simulation has run, as that might be missed from outputs due to when things happen to line up with frequencies, but for anything intermediate, what's the advantage to storing these in addition to the restart files? |
if you want to do a campaign and simulate systems at various T you want configurations consistent with that T. final, shall be saved only if the restart does not coincide with the step at which is write a restart file. |
Ah ok, so the purpose isn't really a restart file in the same sense as we had previously. I think that makes sense though, although I'm not sure it should necessarily be the default behaviour.
I don't think we currently prevent this? Also worth noting that the final and restart files currently store the data with slightly different formats (e.g. final seems to include |
will do |
harveydevereux
force-pushed
the
final_restart
branch
from
c50b95f
to
76c2f68
Compare
I need to re-update the docs now I have swapped the pressure back again Edit: done |
There was a problem hiding this comment.
This will need rebasing now that #120 is merged, sorry for the inconvenience.
Just done a rebase |
_write_restart now takes bool (default False) to handle the case of the final dump. Modifies some tests since pressure now before temperature
_parameter_prefix inplace of passing pressure sync README with new file names both temp range and MD temp in stats and traj test to verify new heating and MD stems when running both.
heating_md_files and heating_md test the default file names and contents with and without the md run, now both using the Stats object
harveydevereux
force-pushed
the
final_restart
branch
from
73a2e83
to
da2b213
Compare
Fixes typos in test_heating_files docstring Co-authored-by: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com>
use += Co-authored-by: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com>
remove ending as line length is no longer not an issue
harveydevereux
force-pushed
the
final_restart
branch
from
8aba808
to
9dd791b
Compare
There was a problem hiding this comment.
I think at some point this module may need to be refactored, so we may revisit some of the interactions e.g. between NPT and NVT_NH, but I think for now this looks good, and particularly having (mostly) settled on the file naming convention we want is perhaps the most important thing.
Thanks, @harveydevereux!
No problem, thanks for keeping my code in check. Refactoring may be a good idea, also for the file length |
The desired behaviour introduced is to produce a restart file (
.xyz) at the end of a heating cycle or md run. As discussed in #108 the user specified restart frequency may not align with the final time step, and this results in a predictable file name for use in automated meta-workflows.Schematically the file stems will be (NPT is currently no working in this PR),
@ElliottKasoar One point that definitely needs addressing is what is desired when we do a heating run and md run, in the same janus run? Does there need to be some indication the md run is post heating for example?
This PR Closes #115