Skip to content

Add superacid HF/SbF5 density benchmark Ref #379#410

Draft
mattiaperr wants to merge 1 commit intoddmms:mainfrom
mattiaperr:superacid-density-benchmark
Draft

Add superacid HF/SbF5 density benchmark Ref #379#410
mattiaperr wants to merge 1 commit intoddmms:mainfrom
mattiaperr:superacid-density-benchmark

Conversation

@mattiaperr
Copy link

@mattiaperr mattiaperr commented Mar 5, 2026

Pre-review checklist for PR author

PR author must check the checkboxes below when creating the PR.

Summary

New benchmark category "Superacids" with an HF/SbF5 liquid density test. NPT simulations at 288.6 K / 1 atm for three compositions (pure HF, 10% SbF5, pure SbF5). Metric is MAPE of predicted density vs experimental values.

Linked issue

Resolves #379

Progress

  • Calculations
  • Analysis
  • Application
  • Documentation

todo:

  • Input data upload to S3 bucket (currently in local cache)
  • Production-length runs (current test runs are short)?
  • Documentation .rst page for superacids category

Testing

Tested on: mace-mp-0a, mace-mp-0b3, mace-mpa-0, mace-omat-0, mace-matpes-r2scan

New decorators/callbacks

No new decorators or callbacks required. Uses existing plot_parity, build_table, and plot_from_table_column.

@ElliottKasoar ElliottKasoar added the new benchmark Proposals and suggestions for new benchmarks label Mar 5, 2026
Copy link
Collaborator

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@@ -0,0 +1,2 @@
title: Superacids
description: Properties of HF/SbF5 superacid systems
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably too specific as an entire category description. Do you think superacids makes sense as an entire category, and if so, could this be made slightly more general?

bad: 10.0
unit: "%"
tooltip: "Mean Absolute Percentage Error in liquid density vs experiment"
level_of_theory: Experiment
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
level_of_theory: Experiment
level_of_theory: Experimental

This is not documented yet, sorry. We need specific key words for this to be interpreted correctly

Comment on lines +1 to +5
outputs/
*.xyz
*.dat
*.log
__pycache__/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these not already ignored by the top-level .gitignore? I haven't seen any of these sorts of files in my git status.

If any of these are necessary, I think they're general enough that we can add them to general one anyway.


MODELS = load_models(current_models)

DATA_PATH = Path(__file__).parent / "data"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DATA_PATH = Path(__file__).parent / "data"

Unused (and probably not wanted, as we tend to download data now)?

Comment on lines +29 to +32
# Unit conversion
EV_TO_KJ_PER_MOL = units.mol / units.kJ


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Unit conversion
EV_TO_KJ_PER_MOL = units.mol / units.kJ

Unused?

hf_sbf5_density_dir = (
download_s3_data(
key="inputs/superacids/HF_SbF5_density/HF_SbF5_density.zip",
filename="HF_SbF5_density.zip",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share this file e.g. on Slack/email so I can upload this please?

Comment on lines +90 to +105
opt = FIRE(atoms, logfile=str(write_dir / "opt.log"))
opt.run(fmax=0.05, steps=N_MIN_STEPS)
write(write_dir / "minimised.xyz", atoms)

MaxwellBoltzmannDistribution(atoms, temperature_K=TEMPERATURE_K)
Stationary(atoms)
ZeroRotation(atoms)

dyn = IsotropicMTKNPT(
atoms=atoms,
timestep=DT,
temperature_K=TEMPERATURE_K,
pressure_au=PRESSURE_AU,
tdamp=TDAMP,
pdamp=PDAMP,
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to change this, but are you aware than janus-core will do a lot of this for you automatically, e.g. the minimisation, setting the initial velocity distribution, saving various stats e.g. the volume etc?

Comment on lines +12 to +19
from ml_peg.analysis.utils.utils import build_d3_name_map, load_metrics_config
from ml_peg.app import APP_ROOT
from ml_peg.calcs import CALCS_ROOT
from ml_peg.models.get_models import get_model_names
from ml_peg.models.models import current_models

MODELS = get_model_names(current_models)
D3_MODEL_NAMES = build_d3_name_map(MODELS)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from ml_peg.analysis.utils.utils import build_d3_name_map, load_metrics_config
from ml_peg.app import APP_ROOT
from ml_peg.calcs import CALCS_ROOT
from ml_peg.models.get_models import get_model_names
from ml_peg.models.models import current_models
MODELS = get_model_names(current_models)
D3_MODEL_NAMES = build_d3_name_map(MODELS)
from ml_peg.analysis.utils.utils import build_dispersion_name_map, load_metrics_config
from ml_peg.app import APP_ROOT
from ml_peg.calcs import CALCS_ROOT
from ml_peg.models.get_models import get_model_names
from ml_peg.models.models import current_models
MODELS = get_model_names(current_models)
DISPERSION_NAME_MAP = build_dispersion_name_map(MODELS)

We recently renamed this sightly. You may need to rebase for this to work pre-merging.

filename=OUT_PATH / "hf_sbf5_density_metrics_table.json",
metric_tooltips=DEFAULT_TOOLTIPS,
thresholds=DEFAULT_THRESHOLDS,
mlip_name_map=D3_MODEL_NAMES,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mlip_name_map=D3_MODEL_NAMES,
mlip_name_map=DISPERSION_NAME_MAP,

Following above



# amu to g conversion factor
AMU_TO_G = 1.66053906660e-24 # g per amu
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can get this from ase.units e.g. 1000 / units.kg to reduce hard-coding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new benchmark Proposals and suggestions for new benchmarks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HF/SbF5 mixture

2 participants