Cu h2o rdf#470
Conversation
|
Thanks for the pr @IsaacParker30 @xradvincula, excited to get these metrics in for everyone to use. Initial thoughts:
code cleanup:
|
|
@joehart2001 is aml still maintained? I know we used in a previous life, @ElliottKasoar can you see what is missing and add them in janus post-processing if aml is not working... |
|
@xradvincula @IsaacParker30 reminder |
|
@xradvincula @IsaacParker30 please also take a look at our new filtering guidelines: https://ddmms.github.io/ml-peg/developer_guide/filter.html Since you have a single system, it should require pretty minimal changes. |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
De-duplicate the three identical per-benchmark aml.py copies by vendoring a single shared copy at ml_peg/analysis/utils/aml_md_analysis.py (PR ddmms#470 review). The per-benchmark copies are removed in a later commit once imports are updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the RDF/VDOS/VACF loader/scorer/bar-builder logic that is byte-identical across the bulk_water, ice and copper_water_interface analyses into a single shared module, parametrised by per-benchmark paths/models (PR ddmms#470 review). The analyse_*.py files are rewritten to delegate here in a later commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidate the cell_to_bar decorator (identical across the three per-benchmark decorators.py copies) into the shared decorator module (PR ddmms#470 review). The histogram decorator is not duplicated: the analyses are switched onto the existing shared plot_hist instead. Local decorators.py copies are removed later. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bulk_water, ice and copper_water_interface analyses shared ~700 lines of byte-identical RDF/VDOS/VACF loader/scorer/bar-builder logic. Rewrite each as config constants + thin pytest fixtures delegating to the shared md_water_analysis module and the shared decorators (PR ddmms#470 review). Copper's dipole histogram is unified onto the existing shared plot_hist: the analysis now keeps raw dipole arrays (which also better serve future structure sampling than the unused pre-binned indices) and lets plot_hist bin them, so the bespoke pre-binned histogram decorator is no longer needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restore docs/.../adding_benchmark.ipynb to main (the branch's tutorial edits were out of scope per the PR ddmms#470 review) and remove the personal CODEBASE_UNDERSTANDING.md ignore line, keeping the benchmark data-dir ignores. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete the three per-benchmark aml.py and decorators.py copies now that their content is vendored/consolidated in ml_peg/analysis/utils, and remove the stray 'et --soft HEAD~1' file accidentally committed earlier (PR ddmms#470 review). All recoverable from git history. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the hand-rolled ASE Langevin loop with janus-core NVT, mirroring the water_slab_dipoles calc. Writes md-traj.extxyz (positions + momenta + masses, every step) instead of md.thermo/md-pos.xyz/md-velc.xyz; the per-frame water dipole is no longer computed during MD (moved to analysis, matscipy dropped). Keeps deuterated H, fixed bottom slab layers, 330 K / friction 0.05 / 1 fs, 5000 equilibration + 30000 production. Sets pbc=[True, True, False] for the slab. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the hand-rolled ASE Langevin loops with janus-core NVT (as done for copper). Each writes md-traj.extxyz (positions + momenta + masses, every step) instead of md.thermo/md-pos.xyz/md-velc.xyz. Preserves bulk_water T=330 K and ice T=250 K, friction 0.05, 1 fs, 25000 equilibration + 300000 production, periodic in all directions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adapt create_rdfs/create_vdos/create_vacf to read the single janus-core md-traj.extxyz (positions + momenta) instead of the legacy md-pos.xyz/md-velc.xyz. New _load_positions/_load_velocities helpers rebuild the mdtraj trajectories with the same Å→nm scaling and init.pdb cell injection the old paths used, so the shipped reference pkls and the aml maths stay valid. Velocities come from atoms.get_velocities() (masses in-file, so deuteration is handled). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ml_peg/analysis/utils/dipoles.get_z_dipoles (point-charge total z-dipole per unit area). water_slab_dipoles now delegates to it (q=0.5562, output unchanged). Copper computes its dipole the same way from the janus md-traj.extxyz with the same q=0.5562 instead of reading a bespoke md.thermo column, keeping the stdev_dipole_z_deviation metric and histogram. NOTE: with the shared charge, copper's ref_dipole_data.npy must be regenerated at q=0.5562 for the deviation to be on a consistent scale (flagged in code). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Report the fraction of MD frames whose total z-dipole per unit area falls outside the stable interfacial band-gap window (below -0.019686563354143947 or above 0.011680694256792076 e/Å), mirroring the water_slab_dipoles "Fraction Breakdown Candidates" metric but with an asymmetric window on the shared q=0.5562 dipole scale. - New fraction_breakdown_candidates fixture reuses the raw_dipoles already computed by stdev_dipole_z_deviation (no extra trajectory read). - Added to the metrics table and metrics.yml (good 0 / bad 1). - App: clicking the new column shows the same dipole distribution histogram. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring the three MD benchmarks up to the element-filtering standard already used by physicality/water_slab_dipoles, so they can be excluded from scoring when a model lacks a required element. - Analysis: each test_* now writes OUT_PATH/info.json via write_struct_info, reading the mock model's md-final.extxyz (the janus final structure). - App: each app defines INFO_PATH = DATA_PATH / "info.json" and passes info_path to its *App constructor so BaseApp loads the elements and filters the metrics table. Calcs already catch MD errors -> NaN and the mock model already runs as part of ml_peg calc, so no calc changes and no extra runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
De-duplicate the three identical per-benchmark aml.py copies by vendoring a single shared copy at ml_peg/analysis/utils/aml_md_analysis.py (PR ddmms#470 review). The per-benchmark copies are removed in a later commit once imports are updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the RDF/VDOS/VACF loader/scorer/bar-builder logic that is byte-identical across the bulk_water, ice and copper_water_interface analyses into a single shared module, parametrised by per-benchmark paths/models (PR ddmms#470 review). The analyse_*.py files are rewritten to delegate here in a later commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidate the cell_to_bar decorator (identical across the three per-benchmark decorators.py copies) into the shared decorator module (PR ddmms#470 review). The histogram decorator is not duplicated: the analyses are switched onto the existing shared plot_hist instead. Local decorators.py copies are removed later. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bulk_water, ice and copper_water_interface analyses shared ~700 lines of byte-identical RDF/VDOS/VACF loader/scorer/bar-builder logic. Rewrite each as config constants + thin pytest fixtures delegating to the shared md_water_analysis module and the shared decorators (PR ddmms#470 review). Copper's dipole histogram is unified onto the existing shared plot_hist: the analysis now keeps raw dipole arrays (which also better serve future structure sampling than the unused pre-binned indices) and lets plot_hist bin them, so the bespoke pre-binned histogram decorator is no longer needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restore docs/.../adding_benchmark.ipynb to main (the branch's tutorial edits were out of scope per the PR ddmms#470 review) and remove the personal CODEBASE_UNDERSTANDING.md ignore line, keeping the benchmark data-dir ignores. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete the three per-benchmark aml.py and decorators.py copies now that their content is vendored/consolidated in ml_peg/analysis/utils, and remove the stray 'et --soft HEAD~1' file accidentally committed earlier (PR ddmms#470 review). All recoverable from git history. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
i guess this is not only for water md analysis so could be renamed more generally
|
Looking really good overall! @IsaacParker30 @xradvincula TODO:
|
7219707 to
bf54094
Compare
There was a problem hiding this comment.
Could you revert this change?
There was a problem hiding this comment.
Yep.
FYI I'm making some bigger changes to add plots from Fig 1 of https://arxiv.org/abs/2603.04228
There was a problem hiding this comment.
Ok nice! Send me a message when you next want me to take a look
Pre-review checklist for PR author
PR author must check the checkboxes below when creating the PR.
Summary
Add's benchmarks for copper-water interface [rdfs,vdos,vacf,dipole fluctuations] and for water/ice [rdfs,vdos,vacf]
Linked issue
Resolves #299 #300
Progress
Testing
Tested on mace-mp-0b3, orb-v3-consv-inf-omat
New decorators/callbacks
Have added new call backs, mainly to make interactive pdf score plots.