From f366916fb1d67f960fc4e625d1e3dd821802f3d9 Mon Sep 17 00:00:00 2001 From: Yury Lysogorskiy Date: Thu, 27 Aug 2026 12:41:43 +0000 Subject: [PATCH 1/3] Deprecate top-level `cutoff`, always deduce it from bonds::rcut The top-level `cutoff` of the input YAML has never reached the fit: it was read in `bin/pacemaker` and required to be present, but was never passed to `GeneralACEFit`, so the `cutoff is None` branch always applied and the neighbour lists were always built with max(bonds::rcut). The last consumer of the value was the `--query-data` path removed in 29ba519. This left a mandatory parameter whose value was silently discarded: users could set it inconsistently with `bonds::rcut` and never learn about it, while omitting it aborted the run with a ValueError. Rather than start honouring the value - which would make an under-sized `cutoff` silently truncate the training neighbour lists, and an over-sized one waste memory for identically zero contributions - the parameter is dropped. Deducing the cutoff from max(bonds::rcut) makes the fitting neighbour lists agree with `ACEBBasisSet.cutoffmax`, which the ASE and LAMMPS calculators use, so the two cannot drift apart. - bin/pacemaker: `cutoff` is no longer required; a deprecation warning is logged if it is still present, so existing input files keep working. - generalfit.py: extract `get_maximal_rcut()` and log the resolved cutoff, so the value actually used is always visible in log.txt. The `cutoff` argument of `GeneralACEFit` is kept as a programmatic escape hatch and documented. - input_template.yaml / `pacemaker -t`: the dialog set both `cutoff` and `bonds::rcut` from one answer; it now asks for `rcut` only and the generated file has no top-level `cutoff`. - examples and CLI tests: drop the redundant `cutoff` line. All eight files had `cutoff == max(bonds::rcut)` exactly, so the deduced value is unchanged. - docs: describe the deduction in inputfile.md, quickstart.md and faq.md. The inputfile.md example was itself inconsistent (`cutoff: 10.0` against rcut 5/5.5). - tests: cover `get_maximal_rcut`, including that it agrees with `ACEBBasisSet.cutoffmax` for per-bond cutoffs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017QbfG2UVKxkeuiz9GtDLo4 --- bin/pacemaker | 13 +++--- docs/pacemaker/faq.md | 3 +- docs/pacemaker/inputfile.md | 10 ++-- docs/pacemaker/quickstart.md | 4 +- examples/Cu-I/input.yaml | 1 - examples/Cu-II/input.yaml | 1 - examples/Ethanol/input.yaml | 1 - examples/HEA/input.yaml | 1 - src/pyace/data/input_template.yaml | 3 +- src/pyace/generalfit.py | 26 +++++++++-- tests/test-CLI/Cu-I/input.yaml | 1 - tests/test-CLI/Cu-I/input_aug.yaml | 1 - tests/test-CLI/Ethanol/input.yaml | 1 - tests/test-CLI/Ethanol/input_aug.yaml | 1 - tests/test_cutoff_deduction.py | 67 +++++++++++++++++++++++++++ 15 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 tests/test_cutoff_deduction.py diff --git a/bin/pacemaker b/bin/pacemaker index ed110b4..0434a2b 100644 --- a/bin/pacemaker +++ b/bin/pacemaker @@ -159,10 +159,9 @@ def main(args): assert isinstance(args_yaml, dict) if "cutoff" in args_yaml: - cutoff = args_yaml["cutoff"] - else: - log.warning("No 'cutoff' provided in YAML file, please specify it") - raise ValueError("No 'cutoff' provided in YAML file, please specify it") + log.warning("Top-level 'cutoff' is deprecated and is ignored. " + "The cutoff for the neighbour list construction is deduced from the potential " + "as the maximum over `potential::bonds::rcut`.") if "seed" in args_yaml: seed = args_yaml["seed"] @@ -325,8 +324,8 @@ def generate_template_input(): """Enter number of functions per element ([ENTER] - default 700): """) or 700) print("Number of functions per element: ", number_of_functions_per_element) - cutoff = float(input("Enter cutoff (Angstrom, default:7.0): ") or 7.0) - print("Cutoff: ", cutoff) + rcut = float(input("Enter outer cutoff `rcut` (Angstrom, default:7.0): ") or 7.0) + print("Outer cutoff rcut: ", rcut) # weighting scheme default_energy_based_weighting = """{ type: EnergyBasedWeightingPolicy, DElow: 1.0, DEup: 10.0, DFup: 50.0, DE: 1.0, DF: 1.0, wlow: 0.75, energy: convex_hull, reftype: all,seed: 42}""" @@ -349,7 +348,7 @@ def generate_template_input(): input_yaml_text = f.read() input_yaml_text = input_yaml_text.replace("{{ELEMENTS}}", str(elements)) - input_yaml_text = input_yaml_text.replace("{{CUTOFF}}", str(cutoff)) + input_yaml_text = input_yaml_text.replace("{{RCUT}}", str(rcut)) input_yaml_text = input_yaml_text.replace("{{DATAFILENAME}}", train_filename) input_yaml_text = input_yaml_text.replace("{{number_of_functions_per_element}}", "number_of_functions_per_element: {}".format( diff --git a/docs/pacemaker/faq.md b/docs/pacemaker/faq.md index fc6d87f..b298046 100644 --- a/docs/pacemaker/faq.md +++ b/docs/pacemaker/faq.md @@ -163,7 +163,8 @@ data: ## I want to change the cutoff, what should I do ? If you change cutoff, i.e. from `rcut: 7` to `rcut: 6.5`, then potential should be refitted from the scratch. -`pacemaker` will recompute neighbourlists on every run, so, no need to extra options except for specifying cutoff. +Change `potential::bonds::rcut` only - the cutoff for the neighbour lists is deduced from it automatically. +`pacemaker` recomputes the neighbour lists on every run, so no extra options are needed. diff --git a/docs/pacemaker/inputfile.md b/docs/pacemaker/inputfile.md index e705847..f7e878c 100644 --- a/docs/pacemaker/inputfile.md +++ b/docs/pacemaker/inputfile.md @@ -5,11 +5,13 @@ particular settings of `pacemaker`. The sections are listed below. ## Cutoff and (optional) metadata -* Global cutoff for the neighborlist constructor is setup as: +* The cutoff for the neighbour list construction is **not** specified separately. It is deduced from the potential + as the maximum of the bonds' outer cutoffs `potential::bonds::rcut` (see the `potential` section below), + which is exactly the cutoff that the ACE calculators (ASE, LAMMPS) use. The atomic environments seen during + fitting are therefore identical to those seen in production, and changing `rcut` is enough - nothing else + has to be kept in sync. -```YAML -cutoff: 10.0 -``` + A top-level `cutoff` key is deprecated: it is ignored and only produces a warning. * Metadata (optional) diff --git a/docs/pacemaker/quickstart.md b/docs/pacemaker/quickstart.md index 30fa786..4e449d0 100644 --- a/docs/pacemaker/quickstart.md +++ b/docs/pacemaker/quickstart.md @@ -34,8 +34,8 @@ In order to interactively generate default `pacemaker` input file `input.yaml`, ``` pacemaker -t ``` -and enter requested information, such as dataset filename, test set size (optional), list of elements, cutoff, -number of functions. Doing so will produce an `input.yaml` file with the most general +and enter requested information, such as dataset filename, test set size (optional), list of elements, +outer cutoff `rcut`, number of functions. Doing so will produce an `input.yaml` file with the most general settings that can be adjusted for a particular task. Detailed overview of the input file parameters can be found in the [Input file](inputfile.md) section below. ## (optional) Manual fitting dataset preparation diff --git a/examples/Cu-I/input.yaml b/examples/Cu-I/input.yaml index 15e525b..6ffbb2a 100644 --- a/examples/Cu-I/input.yaml +++ b/examples/Cu-I/input.yaml @@ -1,4 +1,3 @@ -cutoff: 7 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/examples/Cu-II/input.yaml b/examples/Cu-II/input.yaml index 3c4fb29..53a3ce4 100644 --- a/examples/Cu-II/input.yaml +++ b/examples/Cu-II/input.yaml @@ -1,4 +1,3 @@ -cutoff: 7.5 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/examples/Ethanol/input.yaml b/examples/Ethanol/input.yaml index b4bebef..511dc9f 100644 --- a/examples/Ethanol/input.yaml +++ b/examples/Ethanol/input.yaml @@ -1,4 +1,3 @@ -cutoff: 4 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/examples/HEA/input.yaml b/examples/HEA/input.yaml index e590438..d740525 100644 --- a/examples/HEA/input.yaml +++ b/examples/HEA/input.yaml @@ -1,4 +1,3 @@ -cutoff: 7 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/src/pyace/data/input_template.yaml b/src/pyace/data/input_template.yaml index 1769350..aa66e7b 100644 --- a/src/pyace/data/input_template.yaml +++ b/src/pyace/data/input_template.yaml @@ -1,4 +1,3 @@ -cutoff: {{CUTOFF}} # cutoff for neighbour list construction seed: 42 # random seed ################################################################# @@ -25,7 +24,7 @@ potential: ALL: { radbase: SBessel, radparameters: [ 5.25 ], - rcut: {{CUTOFF}}, + rcut: {{RCUT}}, dcut: 0.01, NameOfCutoffFunction: cos, } diff --git a/src/pyace/generalfit.py b/src/pyace/generalfit.py index a33754f..3dd6bf6 100644 --- a/src/pyace/generalfit.py +++ b/src/pyace/generalfit.py @@ -113,6 +113,19 @@ def get_initial_potential(start_potential): return initial_bbasisconfig +def get_maximal_rcut(bbasisconfig: BBasisConfiguration) -> float: + """ + Maximal outer cutoff `rcut` over all bond (i.e. one- and two-species) blocks. + + This is the same quantity as `ACEBBasisSet.cutoffmax`, which the ACE calculators + (ASE, LAMMPS) use to construct their neighbour lists. Deducing the fitting cutoff + from it guarantees that the atomic environments seen during the fit are identical + to those seen in production. + """ + return max(block.rcutij for block in bbasisconfig.funcspecs_blocks + if len(block.elements_vec) <= 2) + + def save_dataset(dataframe, fname): # columns to save: w_energy, w_forces, NUMBER_OF_ATOMS, PROTOTYPE_NAME, prop_id,structure_id, gen_id, if any # columns_to_save = ["PROTOTYPE_NAME", "NUMBER_OF_ATOMS", "prop_id", "structure_id", "gen_id", "pbc"] + \ @@ -143,6 +156,11 @@ class GeneralACEFit: :param data_config: training data specification :param backend_config: specification of potential evaluation and fitting backend (pyace / tensorpot) - dict ['evaluator'] + :param cutoff: cutoff for the neighbour lists construction. If None (default), it is deduced from + `potential_config` as the maximum over the bonds' outer cutoffs `rcut`, which is what + the ACE calculators use in production. Provide it explicitly only if the neighbour + lists have to extend beyond the potential's own cutoff; note that neighbours + further than the corresponding bond's `rcut` contribute exactly zero to the fit. """ def __init__(self, @@ -265,11 +283,13 @@ def __init__(self, log.info("Ladder-scheme fitting is ON") if cutoff is None: - rcut = max( - [block.rcutij for block in self.target_bbasisconfig.funcspecs_blocks if len(block.elements_vec) <= 2]) - self.cutoff = rcut + self.cutoff = get_maximal_rcut(self.target_bbasisconfig) + log.info("Cutoff for neighbour lists construction is deduced from the potential: " + "{:.3f} A (maximum over bonds::rcut)".format(self.cutoff)) else: self.cutoff = cutoff + log.info("Cutoff for neighbour lists construction is provided explicitly: " + "{:.3f} A".format(self.cutoff)) if self.ladder_scheme: if FIT_LADDER_TYPE_KW in fit_config: diff --git a/tests/test-CLI/Cu-I/input.yaml b/tests/test-CLI/Cu-I/input.yaml index c724257..9108993 100644 --- a/tests/test-CLI/Cu-I/input.yaml +++ b/tests/test-CLI/Cu-I/input.yaml @@ -1,4 +1,3 @@ -cutoff: 7 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/tests/test-CLI/Cu-I/input_aug.yaml b/tests/test-CLI/Cu-I/input_aug.yaml index b6bd9af..a7681ab 100644 --- a/tests/test-CLI/Cu-I/input_aug.yaml +++ b/tests/test-CLI/Cu-I/input_aug.yaml @@ -1,4 +1,3 @@ -cutoff: 7 # cutoff for neighbour list construction seed: 42 # random seed diff --git a/tests/test-CLI/Ethanol/input.yaml b/tests/test-CLI/Ethanol/input.yaml index 85802d8..b9ad562 100644 --- a/tests/test-CLI/Ethanol/input.yaml +++ b/tests/test-CLI/Ethanol/input.yaml @@ -1,4 +1,3 @@ -cutoff: 5.0 # cutoff for neighbour list construction seed: 42 # random seed ################################################################# diff --git a/tests/test-CLI/Ethanol/input_aug.yaml b/tests/test-CLI/Ethanol/input_aug.yaml index a49f7b6..a8e8b80 100644 --- a/tests/test-CLI/Ethanol/input_aug.yaml +++ b/tests/test-CLI/Ethanol/input_aug.yaml @@ -1,4 +1,3 @@ -cutoff: 5.0 # cutoff for neighbour list construction seed: 42 # random seed ################################################################# diff --git a/tests/test_cutoff_deduction.py b/tests/test_cutoff_deduction.py new file mode 100644 index 0000000..620cc0f --- /dev/null +++ b/tests/test_cutoff_deduction.py @@ -0,0 +1,67 @@ +import pytest + +from pyace import ACEBBasisSet +from pyace.basisextension import construct_bbasisconfiguration +from pyace.generalfit import get_maximal_rcut + + +def build_potential_config(bonds): + return { + 'deltaSplineBins': 0.001, + 'elements': ['Al', 'Ni'], + + 'embeddings': {'ALL': {'drho_core_cut': 250, + 'fs_parameters': [1, 1], + 'ndensity': 1, + 'npot': 'FinnisSinclairShiftedScaled', + 'rho_core_cut': 200000}}, + + 'bonds': bonds, + + 'functions': { + 'UNARY': { + 'nradmax_by_orders': [5, 2], + 'lmax_by_orders': [0, 0] + }, + 'BINARY': { + 'nradmax_by_orders': [5, 2, 2], + 'lmax_by_orders': [0, 2, 2], + }, + } + } + + +DEFAULT_BOND = {'NameOfCutoffFunction': 'cos', + 'core-repulsion': [10000.0, 5.0], + 'dcut': 0.01, + 'radbase': 'ChebPow', + 'radparameters': [2.0], + 'lmax': 2, + 'rcut': 3.9} + + +def test_get_maximal_rcut_uniform(): + config = construct_bbasisconfiguration(build_potential_config({'ALL': DEFAULT_BOND})) + assert get_maximal_rcut(config) == pytest.approx(3.9) + + +def test_get_maximal_rcut_per_bond(): + bonds = {'ALL': DEFAULT_BOND, + 'AlNi': dict(DEFAULT_BOND, rcut=5.5)} + config = construct_bbasisconfiguration(build_potential_config(bonds)) + assert get_maximal_rcut(config) == pytest.approx(5.5) + + +@pytest.mark.parametrize("binary_rcut", [3.9, 5.5]) +def test_get_maximal_rcut_equals_cutoffmax(binary_rcut): + """ + The cutoff deduced for the neighbour lists must coincide with `ACEBBasisSet.cutoffmax`, + which is what the ACE calculators (ASE, LAMMPS) use in production. `get_maximal_rcut` + only inspects one- and two-species blocks, while `cutoffmax` is a maximum over all + blocks - these agree because the radial basis (`rcut` included) is copied from the + pair block into the higher-rank blocks. + """ + bonds = {'ALL': DEFAULT_BOND, + 'AlNi': dict(DEFAULT_BOND, rcut=binary_rcut)} + config = construct_bbasisconfiguration(build_potential_config(bonds)) + assert get_maximal_rcut(config) == pytest.approx(ACEBBasisSet(config).cutoffmax) From 3bfa4dfdfb6263332be6ecffe6968f8629f90bec Mon Sep 17 00:00:00 2001 From: Yury Lysogorskiy Date: Thu, 27 Aug 2026 13:57:28 +0000 Subject: [PATCH 2/3] CI: install lib/maxvolpy explicitly so the test suite can be collected Both CI jobs aborted during pytest collection with pyace/activelearning.py:9: from maxvolpy.maxvol import maxvol E ModuleNotFoundError: No module named 'maxvolpy' Interrupted: 2 errors during collection so no test ran at all. This is pre-existing and unrelated to any particular change - run 31264271622 (17 Aug, PR #103) fails identically. `maxvolpy` is installed by the custom InstallMaxVolPyLocalPackage(install) command in setup.py, which shells out to `cd lib/maxvolpy; python setup.py install`. The workflow installs with `pip install .`, which builds a wheel and never invokes the legacy `install` command, so that hook never runs. (`python setup.py install` is also gone in setuptools >= 80.) Build it explicitly instead. `--no-build-isolation` is required because lib/maxvolpy/setup.py imports Cython and numpy at module level while declaring no pyproject.toml build-system, so an isolated build environment fails with ModuleNotFoundError: No module named 'Cython'. Cython and scipy are installed first: the extension cimports scipy/linalg/cython_blas.pxd. numpy is deliberately not installed here, to avoid resolving past setup.py's `numpy<=1.26.4` pin; it is already present from the python-ace install. Verified locally in a clean venv: with build isolation the build fails on the missing Cython, without scipy it fails cythonizing _maxvol.pyx, and with the command used here it builds a wheel and `from maxvolpy.maxvol import maxvol` succeeds. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017QbfG2UVKxkeuiz9GtDLo4 --- .github/workflows/test.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 512f43a..e3d4127 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,6 +42,16 @@ jobs: if [ -f requirements.txt ]; then pip install -r requirements.txt; fi #python setup.py install pip install . + - name: Install maxvolpy + # `pip install .` builds a wheel and never runs setup.py's custom + # InstallMaxVolPyLocalPackage(install) hook, so lib/maxvolpy is not + # installed and the active-learning tests fail to import. Build it + # explicitly; --no-build-isolation is required because + # lib/maxvolpy/setup.py imports Cython and numpy at module level + # without declaring them in a pyproject.toml build-system. + run: | + pip install Cython scipy + pip install --no-build-isolation ./lib/maxvolpy - name: Test with python-ace with tensorpotential run: | python -m pytest tests/ --runtensorpot From d63466faa43481496ee85f1c1530bef91bc23063 Mon Sep 17 00:00:00 2001 From: Yury Lysogorskiy Date: Thu, 27 Aug 2026 18:39:18 +0000 Subject: [PATCH 3/3] tests: import UnitCellFilter from ase.filters, falling back to ase.constraints test_relaxation failed on the Python 3.10 job with ImportError: cannot import name 'UnitCellFilter' from 'ase.constraints' ASE moved UnitCellFilter to ase.filters in 3.23 and has since removed the alias from ase.constraints. `ase` is unpinned in requirements.txt, so the 3.10 job resolves a recent ASE without it, while the 3.9 job - which recent ASE no longer supports - still gets a version that has it. That is why only one of the two jobs failed. Try the new location first and fall back to the old one, so both ASE generations keep working. Verified against ase 3.29: ase.filters.UnitCellFilter resolves and importing it from ase.constraints raises exactly the error above. This failure is unrelated to the cutoff change in this branch; it only became visible once the maxvolpy fix in 3bfa4df let the suite be collected at all. It is the sole remaining failure - the run before this commit reported 289 passed, 1 failed on 3.10 and a fully green 3.9. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017QbfG2UVKxkeuiz9GtDLo4 --- tests/test_PyACECalculator.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_PyACECalculator.py b/tests/test_PyACECalculator.py index 2f71236..8a7d760 100644 --- a/tests/test_PyACECalculator.py +++ b/tests/test_PyACECalculator.py @@ -188,7 +188,11 @@ def test_fcc_stress(): def test_relaxation(): - from ase.constraints import UnitCellFilter + try: + # ASE >= 3.23 + from ase.filters import UnitCellFilter + except ImportError: + from ase.constraints import UnitCellFilter from ase.optimize import QuasiNewton calc = PyACECalculator(basis_set="tests/Al.pbe.rhocore-v2.ace")