Conversation
…/ml-peg into mandy_new_benchmark
ElliottKasoar
added
the
new benchmark
ElliottKasoar
left a comment
ElliottKasoar
left a comment
There was a problem hiding this comment.
Thank for this, @MandyHoffmann, it's looking really good!
Are you able to share (e.g. via slack/onedrive) some full results for this, so I can test the analysis/app more carefully?
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
…es.py Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
…poles.py Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
…poles.py DATA_PATH is not needed because of data download instead Co-authored-by: Joseph Hart <92541539+joehart2001@users.noreply.github.com>
…poles.py Co-authored-by: Joseph Hart <92541539+joehart2001@users.noreply.github.com>
|
Also one small thing, could you remove the data dir @MandyHoffmann as you’ve now added the download |
…es.py Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Calculation of dipole like in https://arxiv.org/abs/2603.04228v1 (charge determined there to match revPBE-D3 water dipole). The threshold for a bad dipole is the value at which a closed bandgap is expected, based on the DFT bandgap in the preprint of revPBE-D3 water.
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Co-authored-by: Elliott Kasoar <45317199+ElliottKasoar@users.noreply.github.com>
Updated the documentation for the filename parameter in the plot function.
ElliottKasoar
left a comment
ElliottKasoar
left a comment
There was a problem hiding this comment.
Thanks again for addressing so many comments, @MandyHoffmann!
Would you be able to run the pre-commit to tidy up a a couple of formatting issues it's complaining about?
The pre-commit is also picking up on the numpy import in the calculation script. Since you don't explicitly call any np functions, it shouldn't be needed. It's only used in the analysis from what I can see.
One final thing (I (almost) promise!) is I think it would be good to consider structure visualisation. I suggest this would be a new PR once this is merged and we've started collecting the real data since we'd be collecting the outputs we need anyway, but do you think it might be interesting to show either some of the trajectory or the final structure?
| ) | ||
| dipoles_unit_area = dipoles / atoms[0].cell[0, 0] / atoms[0].cell[1, 1] | ||
| results[model_name] = dipoles_unit_area | ||
| np.save(model_dir / "dipoles.npy", dipoles_unit_area) |
There was a problem hiding this comment.
What's the benefit of saving these dipole.npy files?
The danger here is if we re-run the calculation after running the analysis for some reason (e.g. with different MD settings, model precision, etc.), when we then run the analysis again, we'd end up using the cached dipoles.npy file and not analyse the new data, I think?
If this part of the analysis is particularly expensive, we could consider moving it to the end of calculation instead, but perhaps we just don't need it?
3 participants
Pre-review checklist for PR author
PR author must check the checkboxes below when creating the PR.
Summary
Physicality test for total dipole distribution of water slabs, checks standard deviation and number of candidates for dielectric breakdown.
Linked issue
Resolves #348
Progress
I think I did everything, but please check! For actually running it, the MD calculation time can be adjusted based on how well sampled you want it; longer simulation times are required for a proper Gaussian-shaped histogram. Similarly, one could consider changing the bins to be even smaller.
Currently, no hover templates are set for the histograms.
Testing
Tested on one of my own water models and the small mace-mp0-a model. Calculations are quite expensive (>12 h for the mace-mp0 one).
New decorators/callbacks
Build a new decorator for histograms, oriented at the scatter plot one, included in this PR. Not sure all the callbacks work properly, but one can see the plots.