coarse-graining / cgnet

learning coarse-grained force fields
BSD 3-Clause "New" or "Revised" License
57 stars 26 forks source link

log simulations and save intermediate numpys #168

Closed brookehus closed 4 years ago

brookehus commented 4 years ago

Development:

SaveTheNumpys

This PR:

TODO:

@nec4 if you have comments on the overall strategy, please let me know! And feel free to look over the code for the logging part.

nec4 commented 4 years ago

Looks great - will take a look!

brookehus commented 4 years ago

ok, so there are 2 ways that save_npys and log are supposed to work: you either: (1) give them an integer 1 or greater, in which case they save that number of files/log that number of steps (or greater, in the case of non-even division), or (2) give them a float between 0 and 1, in which case they save files/log at that % completion.

(2) is a little wonky right now. i've written some tests that pass but i'm not totally sure why. anyway, give this a look, and i'll hopefully be able to approach it with a clearer mind tomorrow!

brookehus commented 4 years ago

the last four tests - i.e. the ones with _float at the end - may be totally meaningless

nec4 commented 4 years ago

Great work! I'll take another look!

nec4 commented 4 years ago

There is a lot here - great work! I have a general question - what is the interplay between _npy_interval and save_interval and log_interval (if any)? self.simulated_traj and the other attribute observables are updated in _save_timepoint() but they are only written to file by the function _save_numpy(), which accesses self.simulated_traj and the other attributes when (t+1) % _npy_interval == 0. Will there ever be an issue from having class attribute observables updated by save_interval but the file saved attributes updated by _npy_interval? I may be overthinking this.

brookehus commented 4 years ago

I think your point is good, and to save everyone including myself right now a lot of trouble, I think I will restrict the numpy interval to be only possible at saving interval points

brookehus commented 4 years ago

@nec4 can you resolve solved conversations?

I need to look at the example notebooks and fix any verbose flags, and then we talked about getting the nan checker to return the iteration at which a nan showed up.

Anything else you want to see? Can you look over the latest code?

Update: fixed example notebooks in f51f82e

brookehus commented 4 years ago

Ok @nec4, in addition to standard code review, can you:

nec4 commented 4 years ago

Sounds good - will take a look now!

nec4 commented 4 years ago

I think this looks more clean than before - great work! Aside from a small typo I can't think of anything else to add/change.

brookehus commented 4 years ago

you can decide whether to merge or wait to play around!

nec4 commented 4 years ago

LGTM!