Add superacids category: HF/SbF5 mixture densities and HF structure factor (Ref #379) - #410
Add superacids category: HF/SbF5 mixture densities and HF structure factor (Ref #379)#410mattiaperr wants to merge 8 commits into
Conversation
ElliottKasoar
left a comment
There was a problem hiding this comment.
Thanks for this, @mattiaperr, this looks great so far! I've left a few minor comments.
If you can share the input data, it would be great to start testing this!
|
As this test involves running mutliple MDs in series, it could be a good idea to change parameterise this like in the liquid densities test when you add the docs could you give an estiamte of how long the MD takes on GPU? if its not too expensive then it may not be woth doing this. Otherwise, the calc, analysis and app seem to work well. For the analysis good and bad thresholds, is a 0% good value realistic due to error associated with the experimental measurements? For the app, we could consider some structure visualisation. as this is a trajectory we could visualise this, but this depends on what you think would be best for your test. Thanks! |
5e1876c to
2d1a3df
Compare
|
thanks! I estimate 6 GPU hours for the pure HF system and 2 GPU hours for the pure SbF5 system, for a model that includes dispersion. I wanted to follow up on this to discuss the structure of the test Given the concern about long simulation times, the whole benchmark could be reformulated as follows:
What do you think? |
…sities/calc_liquid_densities.py
ElliottKasoar
left a comment
There was a problem hiding this comment.
Hi @mattiaperr, thanks again for all the work on this!
In addition to the above comments, please can you take a look at our new filtering guidelines: https://ddmms.github.io/ml-peg/developer_guide/filter.html.
The main things to check are that:
- The calculation is robust to errors (or this is suitably handled in analysis)
- The analysis saves an info.json file along with tables etc
- The analysis handles missing/incomplete data from models (and usually gives them a score of NaN/None)
- The app reads the info.json file
I also wondered if you think there's anything else that would be interesting to visualise, beyond the scatter plot?
Sorry for such a delayed response. I've been working to introduce the element filtering I mention above, and wanted that in place before revisiting things to avoid things getting out of sync, and then I was away for quite a lot of June. Personally I don't think it hurts to estimate the runtime, however quick it is, but I'd say that's definitely long enough that it's worth being clear. Regarding the structure factor, I think it's somewhat up to you. In most cases, I'd say if it requires a different calculation (e.g. an entirely new MD simulation), rather than further analysis on the same data, it would usually be a different benchmark (so separate calc/analysis/app). If it's looking at exactly the same structure, we do have some examples of doing multiple types of calculation (e.g. iron_properties), so as long as it's set up in a way where we can run them individually if necessary (which might just be because one is labelled very slow and one isn't), we could do them all together. I don't think we're overly concerned by the computational cost, we have tests which take days on GPU, I think @joehart2001 was making sure it's set up to run smoothly given the cost and that we document it, so as long as the cost tells us something interesting, I'd be inclined to keep it (but we can structure factor too). |
|
Thank you for your review! The superacids category now has the density benchmark plus a separate one for the structure factor. Main things implemented since the last review:
As we discussed a while back, I used the Python MDANSE library to compute S(q). It is declared in pyproject.toml. One thing to flag on shared code (analysis/utils/decorators.py): I added an Tested on mace-mp-0a, mace-mp-0b3 and mace-mpa-0. If this all looks fine, let me know whether you want to run the full set of models yourselves or whether I should do it. I am also attaching the app JSONs, so you can look at the results: extract it into ml_peg/app/data/ and it creates the superacids/ folder. |
Pre-review checklist for PR author
Summary
test 1) HF/SbF5 mixture densities: NPT molecular dynamics at 288.65 K and 1 atm for three
compositions (pure HF, 10 mol % SbF5, pure SbF5), ~200 atoms each, 100 ps per system.
The density is taken from the average volume of the second half of the run, and the
metric is the MAPE against experimental densities (Shair and Schurig, 1951).
test 2) HF structure factor: a single NPT run of 100 HF molecules at 296 K and 1.2 bar for
50 ps. S(q) iis computed with the python library "MDANSE". Two metrics: the R-factor against experiment
(McLain et al., 2004) up to 4 A^-1, and the error on the position of the first peak.
Linked issue
Resolves #379
Progress
todo:
HF_structure.ziphas to be uploaded to S3Testing
mace-mp-0a, mace-mp-0b3 and mace-mpa-0
New decorators/callbacks
Two changes to
plot_scatterinml_peg/analysis/utils/decorators.py, in their own commit:highlight_rangewas being redrawn once per model, so the shaded band stacked up and the annotation was printed on top of itself. It is now drawn once per figure.hlinesoption to draw dashed horizontal reference lines, used to mark the experimental target density on the density-vs-time plots. It defaults toNone, so no existing plot is affected.Other notes
This benchmark adds
mdanseas a dependency, used for the structure factor.Unit tests were added in
tests/test_superacids.pycovering the restart logic, the density calculation and the R-factor.AI disclosure: I used an AI assistant (Claude) while working on this PR, mainly for refactoring and docstrings. All the physics of the simulations and the design of the workflow in the tests are mine, and I have reviewed every line.