Battery Electrolyte Benchmarks (Inter-Intra, Volume Scans)

[PKourtis]

Pre-review checklist for PR author

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

• I've confirmed the contribution guidelines.

Summary

Inter-Intra: Evaluate model predictions of intra and inter molecular properties on a mix of LIB full electrolyte and neat solvent configs across a range of densities.

Co-authored-by: PKourtis panoskourtis12@gmail.com
Co-authored-by: imagdau i.b.magdau@gmail.com

Volume-Scans: Evaluate model energy predictions across a battery solvent and a battery electrolyte Volume Scan.

Linked issue

Battery Electrolyte Benchmarks (Inter-Intra, Volume Scans) --> Resolves #145

Progress

• Calculations
• Analysis
• Application
• Documentation

The tests require the package aseMolec for their analysis (https://github.com/imagdau/aseMolec.git)

One issue encountered is the Volume Scan structures are not visible when the scatter plot points are clicked. The structure visualiser is launched but the structures do not appear on it.

Testing

Tested on:

• mace-mp-0a
• mace-mp-0b3
• mace-mpa-0
• mace-omat-0
• mace-matpes-r2scan
• orb-v3-consv-inf-omat
• pet-mad
• uma-m-1p1-omat

New decorators/callbacks

Modified the plot_parity decorator function to plot a parity plot per model. More details are on #260

[PKourtis]

Added documentation as well.

[joehart2001]

does ase_ga need to be added as a dependency too?

No module named 'ase_ga'
...
E   ImportError: Cannot import ase_ga.utilities.
E   The ase.ga code has moved to a separate project, ase_ga:
E   https://github.com/dtu-energy/ase-ga .
E   Please install it using e.g. pip install ase-ga.

[joehart2001]

also for inter_intra your app file is app.inter instead of app_inter

[PKourtis]

ase_ga is not needed for this application. No need to add it to the dependencies.

[PKourtis]

Changed the inter_intra app file name to app_inter.

[joehart2001]

ase_ga is not needed for this application. No need to add it to the dependencies.

i get the import error out of the box when running the inter_intra test, when importing anaAtoms:

=============================================================================================== ERRORS ===============================================================================================
_________________________________________________________ ERROR collecting ml_peg/calcs/battery_electrolyte/inter_intra/calc_inter_intra.py __________________________________________________________
ImportError while importing test module '/Users/joehart/Desktop/0_Cambridge/0_MPhil_Scientific_Computing/MPhil_project/mlipx_testing/new_format/mlip-testing/ml_peg/calcs/battery_electrolyte/inter_intra/calc_inter_intra.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.venv/lib/python3.12/site-packages/ase/ga/__init__.py:22: in __getattr__
    module = importlib.import_module(modulename)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/opt/homebrew/Cellar/python@3.12/3.12.9/Frameworks/Python.framework/Versions/3.12/lib/python3.12/importlib/__init__.py:90: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
<frozen importlib._bootstrap>:1387: in _gcd_import
    ???
<frozen importlib._bootstrap>:1360: in _find_and_load
    ???
<frozen importlib._bootstrap>:1310: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:488: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1387: in _gcd_import
    ???
<frozen importlib._bootstrap>:1360: in _find_and_load
    ???
<frozen importlib._bootstrap>:1324: in _find_and_load_unlocked
    ???
E   ModuleNotFoundError: No module named 'ase_ga'

The above exception was the direct cause of the following exception:
.venv/lib/python3.12/site-packages/_pytest/python.py:498: in importtestmodule
    mod = import_path(
.venv/lib/python3.12/site-packages/_pytest/pathlib.py:587: in import_path
    importlib.import_module(module_name)
/opt/homebrew/Cellar/python@3.12/3.12.9/Frameworks/Python.framework/Versions/3.12/lib/python3.12/importlib/__init__.py:90: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
<frozen importlib._bootstrap>:1387: in _gcd_import
    ???
<frozen importlib._bootstrap>:1360: in _find_and_load
    ???
<frozen importlib._bootstrap>:1331: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:935: in _load_unlocked
    ???
.venv/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:186: in exec_module
    exec(co, module.__dict__)
ml_peg/calcs/battery_electrolyte/inter_intra/calc_inter_intra.py:10: in <module>
    from aseMolec import anaAtoms
.venv/lib/python3.12/site-packages/aseMolec/anaAtoms.py:8: in <module>
    from ase.ga.utilities import get_rdf
.venv/lib/python3.12/site-packages/ase/ga/__init__.py:24: in __getattr__
    raise ImportError(f'Cannot import {modulename}.\n{msg}') from err
E   ImportError: Cannot import ase_ga.utilities.
E   The ase.ga code has moved to a separate project, ase_ga:
E   https://github.com/dtu-energy/ase-ga .
E   Please install it using e.g. pip install ase-ga.
E   Please import from ase_ga what would previously be imported from ase.ga.
E   ase.ga placeholders will be removed in a future release.

[PKourtis]

Fixed the anaAtoms import erorr. aseMolec now has ase-ga as a dependency so it will be installed automatically with aseMolec. No need to add it as a dependency here.

[joehart2001]

Hi have you pushed your latest version? did you add my code suggestions which fixes the visualisation?

[joehart2001]

Hey @PKourtis is it ok to remove the data files now?

[PKourtis]

Hey @joehart2001 could you add the data files on S3 bucket? I can edit the code to download it directly from there

[ElliottKasoar]

Hey @joehart2001 could you add the data files on S3 bucket? I can edit the code to download it directly from there

We will do shortly, but in the meantime, if you change the code, you can place the relevant data directory in ~/.cache/ml_peg, and the test should work as if you downloaded it, so you should be able to test it reasonably well.

[joehart2001]

hey @PKourtis i realised i had already uplaoded them. so unless anyting has changed, you can find them at:

    data_path = (
        download_s3_data(
            key="inputs/battery_electrolyte/inter_intra/inter_intra.zip",
            filename="inter_intra.zip",
        )
    )

    data_path = (
        download_s3_data(
            key="inputs/battery_electrolyte/volume_scans/volume_scans.zip",
            filename="volume_scans.zip",
        )
    )

[joehart2001]

@PKourtis @ElliottKasoar is this ready (minus the conflicts)?

[ElliottKasoar]

Hi @PKourtis, thanks again for this, and for the latest round of changes!

I've pushed a few minor fixes, and left a couple of quick questions.

I also had a couple more requests:

• I updated the volume scans analysis to not fail when data is missing, but could you make sure the inter_intra analysis also runs with missing data?
• Might it be possible to visualise the reference volume scans, as well as the predicted version? Currently, whichever line you click, it shows the predicted version
• Can you add structure visualisation to the density plots? I think any other example you can find for density plots should have this

[ElliottKasoar]

Is this supposed to be high? We normally use lower precision for MD, and I thought for the paper it's what we used?

[ElliottKasoar]

Is it obvious why this doesn't include intra-energy?

[ElliottKasoar]

Apologies if we've discussed this, but how much have you considered the thresholds here and for the volume scans?