search

espressomd / espresso

The ESPResSo package
https://espressomd.org
GNU General Public License v3.0
230 stars 187 forks source link

Add MLIP Tutorial #4982

Closed PythonFZ closed 3 weeks ago

PythonFZ commented 3 months ago

Add a Tutorial on Machine-Learning Interatomic Potentials.

review-notebook-app[bot] commented 3 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

RudolfWeeber commented 3 months ago

Thank you, Fabian. @SamTov agreed to do a detailed review next week.

Whlie having a quick look, I noticed the following:

PythonFZ commented 2 months ago

Hey Rudolf,

Thanks for the feedback. I have added some more clarification w.r.t. to the code that is used and added some more sources. I'd appreciate if you could help me improve which parts still need more clarification.

  • Units: It wans't completely clear to me at first glance what units are used in the simulatoion. Is ESPResSo run in the usual reduced units?

I have talked to @reinaual about the units and if I understood it correctly, I am defining the unit system in ESPResSo such that it resembles the one used within ASE.

  • As long as there are only external forces, the Verlet-Lists and skin can//should eb switche off

As I am not an ESPResSo user, would it be possible to give me an example or edit the file direclty.

  • It would be important to get the external depencies stable, as we will have to include them in the CI

I have removed the ipsuite dependency because all the features required are within the rdkit2ase package. The new dependencies would be:

SamTov commented 2 months ago

Let us consider what we want from the tutorial. Is a reaction the only analysis we want to do? Don't most people want to run MD before doing a reaction study? Both are nice, but we could do RDF calculations or even Rg computations on something like ethanol. Or go for a bigger molecular and do a quick polymer study.

SamTov commented 2 months ago

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

PythonFZ commented 2 months ago

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

I see - I opened https://github.com/espressomd/espresso/issues/4993 which would resolve this and make it work.

SamTov commented 2 months ago

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

I see - I opened #4993 which would resolve this and make it work.

Is it necessary? They can plot the plotly figure in the notebook and the syntax to have it opened in ZnDraw is quite odd, particularly for those who aren't familiar with the program.

PythonFZ commented 2 months ago

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

Is there any reason you removed the interactive plotting part for the visualizer vis.zndraw.figures = {"distance": fig.to_json()}?

It didn't work when we ran it on the CIP pool computers.

I see - I opened #4993 which would resolve this and make it work.

Is it necessary? They can plot the plotly figure in the notebook and the syntax to have it opened in ZnDraw is quite odd, particularly for those who aren't familiar with the program.

PythonFZ commented 2 months ago

Thanks for all the work you put into this 👍

The

#TODO: figure out if this works

as well as the vis.zndraw.figures = {"distance": fig.to_json()} might both depend on https://github.com/espressomd/espresso/issues/4993

Unfortunately, I don't know what issues came up with this version and they did not mention or make any issues on the ZnDraw repo. If the issue will not be resolved before the tutorial it might be easiest to remove this part.

RudolfWeeber commented 1 month ago

Is something till missing or can ew go ahed?

SamTov commented 1 month ago

The ground state energies for the water study are resolved and correct.