Cu h2o rdf#470
Draft
IsaacParker30 wants to merge 19 commits into
Draft
Conversation
Collaborator
|
Thanks for the pr @IsaacParker30 @xradvincula, excited to get these metrics in for everyone to use. Initial thoughts:
code cleanup:
|
Collaborator
|
@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... |
Collaborator
|
@xradvincula @IsaacParker30 reminder |
Collaborator
|
@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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.