feat: split molecular liquids into RDF + density benchmarks by lwalew · Pull Request #149 · instadeepai/mlipaudit

lwalew · 2026-07-02T14:32:59Z

Description

Stacked on #140. Splits the two Molecular Liquids benchmarks into four scored benchmarks — water/solvent × RDF/density — implementing the plan agreed in the Slack thread (each score counts independently towards the overall model average, grouped under the existing Molecular Liquids category, no separate landing page).

benchmark	score
`water_radial_distribution`	RDF (peak deviation + RMSE)
`water_density` (new)	relative density deviation
`solvent_radial_distribution`	RDF (mean per-solvent peak deviation)
`solvent_density` (new)	mean per-solvent relative density deviation

Key points

One simulation per group: the RDF and density benchmark of each system group share a reusable_output_id and identical ModelOutput field sets, so the NPT simulation runs only once when both are run together (verified: engine instantiated once across the pair).
Density is now scored on its own via compute_metric_score(density_deviation / reference_density, threshold, alpha), rather than only shown in the RDF UI table.
Shared simulation config, data loaders and density helpers live in utils/molecular_liquids.py.
New Benchmark.data_name lets the density benchmarks reuse the RDF input data (no duplicate HuggingFace uploads).
New density UI pages plot the density time series against the reference.
Docs: added a density benchmark page; moved the RDF api-reference docs under api_reference/molecular_liquids/.

Density scoring

Follows Leon's NPT density analysis: DENSITY_RELATIVE_DEVIATION_THRESHOLD = 0.02 (2% relative deviation) with a gentle DENSITY_SCORE_ALPHA = 0.1 (deliberately not the steep global scoring.ALPHA = 3.0). On the existing NPT runs these reproduce the expected ranking (v1 models best on water; v2 models poor; solvent scores dragged down by the near-universal CCl₄ blow-up).

⚠️ Still pending final sign-off (@S-Gutierrez / @m-carobene / @l-wehrhan): the ideal per-solvent threshold should ultimately be based on the isothermal compressibility of each solvent — 2% is Leon's interim target.

Checklist for adding a new benchmark

Benchmark class is fully implemented, including all abstract method implementations.
Benchmark is fully tested following our standard testing pattern.
Input data uploaded to HF is validated and correct. (density benchmarks reuse the existing RDF data via data_name — no new upload needed)
The corresponding documentation is added and up to date.
The UI code is added and up-to-date and has been tested.
Our license has been added to any Python file.

Notes

Base is feat/npt-sims-solvent (feat: use NPT sims for water + solvent benchmarks #140); retarget to develop once feat: use NPT sims for water + solvent benchmarks #140 merges.
Going 2→4 benchmarks doubles Molecular Liquids' weight in the flat overall-score mean (intended per the thread); the per-category leaderboard column auto-normalizes.

🤖 Generated with Claude Code

Split the water and solvent Molecular Liquids benchmarks into four scored benchmarks: water/solvent x RDF/density. Each is its own Benchmark subclass, score, leaderboard column and UI page, grouped under the "Molecular Liquids" category. - The RDF and density benchmark of each system group share a single NPT simulation via `reusable_output_id` and identical `ModelOutput` field sets, so the simulation is only run once when both are run together. - Density is now scored on its own (relative deviation from the reference density via `compute_metric_score`), rather than only shown in the RDF UI. - Shared simulation config, data loaders and density helpers moved to `utils/molecular_liquids.py`. - New `Benchmark.data_name` lets the density benchmarks reuse the RDF input data without duplicating HuggingFace uploads. - New density UI pages plot the density time series against the reference. - Docs: added a density benchmark page and moved the RDF api-reference docs under `api_reference/molecular_liquids/`. Note: the density score threshold (DENSITY_RELATIVE_DEVIATION_THRESHOLD) is a placeholder pending sign-off from the science team. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-07-02T14:34:08Z

Tests	Skipped	Failures	Errors	Time
139	0 💤	0 ❌	0 🔥	23.035s ⏱️

`Benchmark.__init__` already coerces `run_mode` to a `RunMode`, so annotate the stored attribute as `RunMode` and let the molecular-liquids helpers take a plain `RunMode`. Removes the unnecessary `_as_run_mode` normalization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Match the density score to Leon's NPT density analysis: relative-deviation threshold of 0.02 and a gentle decay (DENSITY_SCORE_ALPHA=0.1), rather than the placeholder 0.05 with the steep global scoring.ALPHA=3.0. With these values the density scores reproduce the expected ranking (v1 models best on water, v2 models poor). The ideal per-solvent threshold should ultimately be based on the isothermal compressibility; still pending final sign-off. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mcarobene-id · 2026-07-06T12:51:42Z

+**NPT** simulation using the **MLIP** model for **500,000 steps**, leveraging the
+`jax-md <https://github.com/google/jax-md>`_ engine from the
+`mlip <https://github.com/instadeepai/mlip>`_ library. Water is run at **295.15 K** and **1 atm**,
+while all other solvents are run at **293.15 K** and **1 atm**. Because the RDF and density


First use of "RDF" here. I guess above we should have

as the :ref:`radial_distribution` (RDF) benchmark

mcarobene-id · 2026-07-06T12:52:40Z

+Reference densities are the experimental values at the simulation conditions: water
+:math:`0.9978\ \text{g/cm}^3`, CCl4 :math:`1.594\ \text{g/cm}^3`, methanol
+:math:`0.791\ \text{g/cm}^3` and acetonitrile :math:`0.786\ \text{g/cm}^3`.


@leonwehrhan We need to add a source

mcarobene-id · 2026-07-06T13:12:36Z

+"""Shared configuration and helpers for the molecular liquids benchmarks.
+
+The water and solvent radial-distribution and density benchmarks all run the same
+NPT simulation of a box of molecules. To avoid running these simulations twice, the
+RDF and density benchmarks of a given system group share a `reusable_output_id` and
+identical `ModelOutput` field signatures (see `benchmarks_cli._transfer_model_output`).
+This module centralizes the simulation configuration, input-data loading and density
+calculation so that both benchmarks stay in sync.
+"""


Possibly remove or simplify?

mcarobene-id · 2026-07-06T13:14:04Z

+#: Per-solvent molecular weight (g/mol) and number of atoms per molecule. The dict
+#: order also defines the canonical order of the solvent systems.


Dict order not important

Suggested change

#: Per-solvent molecular weight (g/mol) and number of atoms per molecule. The dict

#: order also defines the canonical order of the solvent systems.

#: Per-solvent molecular weight (g/mol) and number of atoms per molecule.

Not quite true with the current implementation. In DEV Mode we only run a single system + the UI also hardcodes the order. I'll change that though.

mcarobene-id · 2026-07-06T13:16:08Z

+    return WATER_SIMULATION_CONFIG
+
+
+def load_water_box(data_dir: str | os.PathLike) -> Atoms:


Suggested change

def load_water_box(data_dir: str | os.PathLike) -> Atoms:

def load_water_system(data_dir: str | os.PathLike) -> Atoms:

mcarobene-id · 2026-07-06T13:17:10Z

+    return run_simulation(
+        atoms=load_water_box(data_dir),
+        force_field=force_field,
+        md_integrator=MDIntegrator.NPT_MC_LANGEVIN,
+        molecule_indices=load_water_molecule_indices(data_dir),
+        **get_water_md_kwargs(run_mode),


These methods are only used here - can we make them private to this file

Suggested change

return run_simulation(

atoms=load_water_box(data_dir),

force_field=force_field,

md_integrator=MDIntegrator.NPT_MC_LANGEVIN,

molecule_indices=load_water_molecule_indices(data_dir),

**get_water_md_kwargs(run_mode),

return run_simulation(

atoms=_load_water_box(data_dir),

force_field=force_field,

md_integrator=MDIntegrator.NPT_MC_LANGEVIN,

molecule_indices=_load_water_molecule_indices(data_dir),

**_get_water_md_kwargs(run_mode),

mcarobene-id · 2026-07-06T13:17:48Z

+    system_names = get_solvent_system_names(run_mode)
+    md_kwargs = get_solvent_md_kwargs(run_mode)
+
+    simulation_states: list[SimulationState | None] = []
+    for system_name in system_names:
+        logger.info("Running NPT simulation for %s.", system_name)
+        simulation_state = run_simulation(
+            atoms=load_solvent_system(data_dir, system_name),
+            force_field=force_field,
+            md_integrator=MDIntegrator.NPT_MC_LANGEVIN,
+            molecule_indices=load_solvent_molecule_indices(data_dir, system_name),
+            **md_kwargs,
+        )
+        simulation_states.append(simulation_state)


Same again RE making these methods private

Suggested change

system_names = get_solvent_system_names(run_mode)

md_kwargs = get_solvent_md_kwargs(run_mode)

simulation_states: list[SimulationState | None] = []

for system_name in system_names:

logger.info("Running NPT simulation for %s.", system_name)

simulation_state = run_simulation(

atoms=load_solvent_system(data_dir, system_name),

force_field=force_field,

md_integrator=MDIntegrator.NPT_MC_LANGEVIN,

molecule_indices=load_solvent_molecule_indices(data_dir, system_name),

**md_kwargs,

)

simulation_states.append(simulation_state)

system_names = _get_solvent_system_names(run_mode)

md_kwargs = _get_solvent_md_kwargs(run_mode)

simulation_states: list[SimulationState | None] = []

for system_name in system_names:

logger.info("Running NPT simulation for %s.", system_name)

simulation_state = run_simulation(

atoms=_load_solvent_system(data_dir, system_name),

force_field=force_field,

md_integrator=MDIntegrator.NPT_MC_LANGEVIN,

molecule_indices=_load_solvent_molecule_indices(data_dir, system_name),

**md_kwargs,

)

simulation_states.append(simulation_state)

mcarobene-id · 2026-07-06T13:20:09Z

+# --------------------------------------------------------------------------------------
+# Water data loading + simulation
+# --------------------------------------------------------------------------------------


I think it might be possible to remove these grouping headers once we've made the small helpers private as suggested below - should be easy-ish to tell where each is used as you go down the page

mcarobene-id · 2026-07-06T13:21:13Z

+        Uses `data_name` when set, otherwise `name`, so that benchmarks can share
+        input data (e.g. an RDF and a density benchmark running the same system).


nitpick: generalise

Suggested change

Uses `data_name` when set, otherwise `name`, so that benchmarks can share

input data (e.g. an RDF and a density benchmark running the same system).

Uses `data_name` when set, so that benchmarks can share input data. Otherwise uses `name`.

mcarobene-id

This is really great, thanks @lwalew

I'm overall very happy with this, I just have a couple of small comments RE docstrings + comments.

Also a couple of small Qs:

It's mentioned in a docstring that if an RDF + Density benchmark are run together then the sim is only run once (great). Is this is also true if we e.g. run one in one run, and then later run the other? I know there's outputs+results caching by default so just double-checking this applies here
Is it possible to generate a version of the results pages with these new results pages? Would love to see the layouts of these new pages and view the density plots etc. before approving if possible :)

lwalew · 2026-07-08T11:30:46Z

@mcarobene-id For the 2nd remark, yes let me add those! For the first part as you say it's really an in-memory mechanism we have. We can reuse the output if running the two benchmarks as separate jobs but it needs a bit of extra logic to get it up and running. You need to persist the model_output.zip in the 1st job, modifying run_model() to do so. Then in the 2nd exp, modify the main script a bit with load_model_output_from_disk first to pull the previous trajectories. We could modify our script to do so but I think in this instance it would be easier to rewrite a small script to do the job.
TLDR: Definitely easiest to do both in the same job!

…mment Address review feedback that solvent dict order isn't meaningful. The solvent UI pages no longer hardcode ["CCl4", "methanol", "acetonitrile"] or index `structures` positionally; they derive the system list from the results and look structures up by name (new `ordered_structure_names` helper). Also drop the misleading "canonical order" comment/docstring, since scoring is entirely by-name and nothing depends on the config dict ordering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Docs and make some functions private

claude · 2026-07-08T14:46:05Z

Claude finished @lwalew's task in 3m 2s —— View job

Code review: split molecular liquids into RDF + density

Reviewed against origin/feat/npt-sims-solvent. Overall this is a clean, well-structured split — the shared-simulation reuse via reusable_output_id + data_name is elegant, the name-based UI lookups fix the previous positional-index fragility, tests follow the standard pattern (including the reuse-path test), and license headers + docstrings are all present. A few small things below.

Findings

1. CHANGELOG.md not updated (medium)
This is a user-visible change (2 → 4 scored benchmarks, new density scoring, doubled Molecular Liquids weight), but CHANGELOG.md has no entry — the repo convention (and the review checklist) calls for one. Worth a bullet under the next release describing the split and the new water_density / solvent_density benchmarks. Fix this →

2. Dead defensive guard in score aggregation (nit)
In solvent_density.py:218-220 (and the mirrored solvent_radial_distribution.py:271-273):

score=statistics.mean(
    r.score if r.score is not None else 0.0 for r in structure_results
)

SolventDensityStructureResult.score is typed float = 0.0 and is never None, so the is not None else 0.0 branch is unreachable. Harmless, but statistics.mean(r.score for r in structure_results) is clearer. Same pattern in the RDF benchmark, so feel free to leave for consistency.

3. Reference-density source still missing (doc, open)
docs/source/benchmarks/molecular_liquids/density.rst:47 lists the experimental reference densities but cites no source — this matches @mcarobene-id's earlier "we need to add a source" comment, which still looks open (pending @leonwehrhan). Just flagging it isn't lost.

4. data_dir docstring nit
benchmark.py:236-241 reads well now. One micro-point: the data_name attribute docstring (lines 96-99) mentions the "HuggingFace archive" but the data_dir property docstring only mentions the local dir — both are accurate, just noting the download path (_download_data) correctly uses data_name too, so the sharing works end-to-end. No change needed.

Answers to the two open review questions

Q1 — is the sim reused if RDF and density run in separate runs?

No — reuse is in-memory only, scoped to a single run_benchmarks invocation. In benchmarks_cli.py, reusable_model_outputs is created fresh inside the per-model loop and populated by run_model(); a benchmark only skips its simulation if a same-run sibling with a matching reusable_output_id already populated it (benchmarks_cli.py:301-317). Run the two as separate jobs and the NPT simulation executes again. This confirms your Slack answer — running the RDF + density pair together in one job is the only path that runs the sim once. (Results caching is a separate mechanism and doesn't transfer a sibling benchmark's ModelOutput.)

Q2 — can we see rendered result pages / density plots before approving?

That's outside what I can produce from a static review (needs real NPT trajectories + a running Streamlit app). The page code itself looks correct: water_density.py / solvent_density.py build the summary table and a per-frame density time series with a dashed reference line, and ordered_structure_names (ui/utils.py:329) makes the solvent page robust to system ordering. Rendering a sample would need someone to run the app on cached outputs.

Correctness spot-checks (all pass)

compute_densities units: M·N_mol / (units.mol · V · 1e-24) → g/cm³ is dimensionally correct (units.mol = N_A, Å³→cm³ = 1e-24). ✓
average_equilibrated_density discards the first 1/5 of frames; safe for the 2-frame DEV/mocked case (2//5 = 0). ✓
ModelOutput field sets match across each RDF/density pair, so _transfer_model_output works in either run order. ✓
Reference densities in the docstring match SOLVENT_REFERENCE_DENSITIES / WATER_REFERENCE_DENSITY. ✓

Nice work overall — the only thing I'd treat as a real to-do before merge is the CHANGELOG entry.

lwalew · 2026-07-08T15:14:40Z

@mcarobene-id Just ran ViSNet locally on DEV:
<img width="1641" height="964" alt="Screenshot 2026-07-08 at 17 12 0

3" src="https://github.com/user-attachments/assets/688eb405-7877-4bf3-b089-4868e0

c03f68" />

…nt score mean - Add a Release 0.1.5 CHANGELOG entry describing the RDF/density split, the new water_density / solvent_density benchmarks, the shared-simulation reuse, and the new Benchmark.data_name mechanism. - Drop the unreachable `r.score if r.score is not None else 0.0` guard in the two solvent benchmarks' score aggregation; the per-structure `score` field defaults to 0.0 and is never None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude · 2026-07-08T15:34:54Z

Claude finished @lwalew's task in 2m 10s —— View job

Re-review: split molecular liquids into RDF + density

Re-checked the latest commits against origin/feat/npt-sims-solvent and looked at the rendered pages you shared.

Review new commits since last review (CHANGELOG, references, private helpers, docstrings)
Verify prior findings addressed
Review rendered UI pages shared
Post updated review

Prior findings — status

#	Finding	Status
1	`CHANGELOG.md` not updated	✅ Fixed — clear `Release 0.1.5` entry covering the 4-way split, `data_name`, and the new density UI pages (`CHANGELOG.md:3-12`)
2	Dead `is not None else 0.0` guard in score mean	✅ Fixed — now `statistics.mean(r.score for r in structure_results)` in both `solvent_density.py:218` and `solvent_radial_distribution.py:271`
3	Reference-density source missing	⚠️ Still open — see below
4	`data_dir` / `data_name` docstring	✅ Fixed — generalized to "so that benchmarks can share input data" (`benchmark.py:238-239`)

mcarobene-id's review comments — status

Private helpers — ✅ done: _load_water_system, _load_water_molecule_indices, _get_water_md_kwargs, _load_solvent_system, _load_solvent_molecule_indices, _get_solvent_md_kwargs are all now file-private. The few that remain public (get_solvent_system_names, get_solvent_pdb_file_name, get_solvent_molecule_indices_file_name) are correct to keep — they're imported by the benchmark/UI modules.
SOLVENT_MOLECULE_CONFIG comment / dict-order — ✅ applied (molecular_liquids.py:84).
UI look-up by structure name (ordered_structure_names) — ✅ in place; robust to DEV-mode single-system runs and system ordering.
RDF references — ✅ added (Skinner / Handgraaf / Pothoczki / Nishikawa, radial_distribution.rst:74-80).

Remaining open item (not blocking, needs a domain owner)

Reference-density source (doc) — density.rst:45-47 still lists the experimental densities (water 0.9978, CCl₄ 1.594, methanol 0.791, acetonitrile 0.786 g/cm³) with no citation, unlike the RDF page which now has a full References section. This matches @mcarobene-id's earlier "we need to add a source" comment and is still pending @leonwehrhan. Worth adding a short References section here for parity. Fix this →

Rendered pages

The Solvent density page renders as intended: summary table (score + avg/per-solvent deviations) plus a per-frame density time series with the dashed reference line — the CCl₄ plot clearly shows the model sitting ~0.22 g/cm³ below reference, exactly the blow-up the scoring is designed to catch. The Solvent RDF page shows g(r) with the reference-peak vertical line. Both look correct. ✓

Verdict

All actionable findings from my previous review are addressed, and the new commits are clean. The only thing still outstanding is the density reference source, which is a doc citation pending a domain owner and not a code blocker. Nice work. 👍

lwalew requested review from lwehrhan and mcarobene-id July 3, 2026 14:11

lwalew and others added 3 commits July 3, 2026 16:19

chore: drop verbose comment on density scoring constants

ce664a6

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mcarobene-id reviewed Jul 6, 2026

View reviewed changes

lwalew and others added 4 commits July 8, 2026 15:07

feat: address comments

49cee31

Docs and make some functions private

feat: remove file divider

338866e

feat: update data_dir docstring

ed09f71

lwalew added the claude-review Automatically request a review from Claude label Jul 8, 2026

lwalew added 2 commits July 8, 2026 16:38

feat: more private functions

1b10eaf

docs: add references

a8c9141

This comment has been minimized.

Sign in to view

docs: shorten release

95a8be7

		#: Per-solvent molecular weight (g/mol) and number of atoms per molecule. The dict
		#: order also defines the canonical order of the solvent systems.

		return WATER_SIMULATION_CONFIG


		def load_water_box(data_dir: str \| os.PathLike) -> Atoms:

	def load_water_box(data_dir: str \| os.PathLike) -> Atoms:
	def load_water_system(data_dir: str \| os.PathLike) -> Atoms:

		Uses `data_name` when set, otherwise `name`, so that benchmarks can share
		input data (e.g. an RDF and a density benchmark running the same system).

	Uses `data_name` when set, otherwise `name`, so that benchmarks can share
	input data (e.g. an RDF and a density benchmark running the same system).
	Uses `data_name` when set, so that benchmarks can share input data. Otherwise uses `name`.

Uh oh!

Conversation

lwalew commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Key points

Density scoring

Checklist for adding a new benchmark

Notes

Uh oh!

github-actions Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcarobene-id Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcarobene-id left a comment

Choose a reason for hiding this comment

Uh oh!

lwalew commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review: split molecular liquids into RDF + density

Findings

Answers to the two open review questions

Correctness spot-checks (all pass)

Uh oh!

lwalew commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

claude Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-review: split molecular liquids into RDF + density

Prior findings — status

mcarobene-id's review comments — status

Remaining open item (not blocking, needs a domain owner)

Rendered pages

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lwalew commented Jul 2, 2026 •

edited

Loading

github-actions Bot commented Jul 2, 2026 •

edited

Loading

mcarobene-id Jul 6, 2026 •

edited

Loading

lwalew commented Jul 8, 2026 •

edited

Loading

claude Bot commented Jul 8, 2026 •

edited

Loading

lwalew commented Jul 8, 2026 •

edited

Loading

claude Bot commented Jul 8, 2026 •

edited

Loading