API inconsistencies when saving and loading model snapshots

[elcorto]

• Runner: save_run() has a save_path keyword, which is called path in load_run()
• ASE calculator mala.MALA's load method is called load_model() (not load_run())

[RandomDefaultUser]

As for the path inconsistency I totally agree, that was just human oversight. I added a fix in this small bugfix PR here #579

The second inconsistency is sort of by design. My intuition (which may be completely off here) was that the Runner, Trainer, etc. classes will mostly be used by people with some familiarity in the ML field, which may train/run their models in a variety of settings. In contrast, the ASE class tackles potential users who just want to use MALA. Ideally, at some point in the future, someone would download a pretrained MALA model and then load it with the ASE class and do simulations with it. Hence I opted for load_model to reflect this difference in use case. Do you think that makes sense or does that unnecessarily complicate the API?

[elcorto]

As for the path bit: thanks :)

ASE calculator: In general, if things do the same, then it helps if they have the same name. Here it is a bit different since they load the same data but return different things, but this difference can be clear from context (ASE calc or not). The methods SomeClass.load_run and MALA.load_model take the same args (load_model() only a subset but OK). Even though they return different things, the use case is the same: load a (trained) model plus metadata from disk. I understand the motivation you described, but I think in this case having a common name makes the API cleaner since you point them to the same disk data. Having different names may imply that they load different things, i.e. that in mala there is a thing called "run" and another called "model" which people can load.

As for which name, I'd probably stick with load_run but I that's only a 60/40 opinion. In any case, if you decide to rename, for exmaple, MALA.load_model() to load_run, I'd keep load_model, add a DeprecationWarning and then call load_run.

[RandomDefaultUser]

Yeah, I see your point, and would generally tend to agree. Since there currently is no real user base for our ASE calculator, I don't think it hurts to rename it for now. If we eventually realize, through pain staking market research, that this is what keeps people from using the ASE calculator, we can always reconsider. I will prepare a PR.