diff --git a/tests/calculators/test_aims.py b/tests/calculators/test_aims.py index af3fb4e7..1a2dab89 100644 --- a/tests/calculators/test_aims.py +++ b/tests/calculators/test_aims.py @@ -153,7 +153,7 @@ def test_generic_aims_calculation(tmp_path, parameters_nonperiodic): # single atoms tests assert "Aims_energy" in si_single.info assert "Aims_stress" not in si_single.info - assert si_single.info["Aims_energy"] == pytest.approx(expected=-7869.54379922653, abs=1e-2) + assert si_single.info["Aims_energy"] == pytest.approx(expected=-7869.54379922653, abs=2e-2) assert si_single.get_volume() == pytest.approx(6.0 ** 3) # bulk Si tests diff --git a/tests/calculators/test_orca.py b/tests/calculators/test_orca.py index 91190258..b486171c 100644 --- a/tests/calculators/test_orca.py +++ b/tests/calculators/test_orca.py @@ -22,6 +22,7 @@ from ase import Atoms from ase.calculators.calculator import CalculationFailed from ase.io import orca as orca_io +from ase.io import ParseError from wfl.calculators.orca import ORCA, parse_npa_output, natural_population_analysis from wfl.calculators import generic @@ -60,7 +61,7 @@ def test_orca_is_converged(tmp_path): at.calc = orca # todo - make this run in tmpdir - with pytest.raises(CalculationFailed): + with pytest.raises(ParseError): at.get_potential_energy() @orca_prerequisites diff --git a/tests/test_utils.py b/tests/test_utils.py index c065960c..12609101 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -79,8 +79,8 @@ def test_save_results_no_old_props(): if k.startswith("stored_prop_"): found_n += 1 assert "_md_" in k - # each config should have energy, stress, forces, energies - assert found_n == 4 * len(list(ci_md_1)) + # each config should have energy, stress, forces, energies and free_energy + assert found_n == 5 * len(list(ci_md_1)) ci_md_2 = md(ci_md_1, OutputSpec(), calc, steps=2, dt=1.0, temperature=300, temperature_tau=10.0, rng=rng) found_n = 0 @@ -90,7 +90,7 @@ def test_save_results_no_old_props(): if k.startswith("last_op__"): assert "_md_" in k found_n += 1 - assert found_n == 4 * len(list(ci_md_2)) + assert found_n == 5 * len(list(ci_md_2)) ci_opt = optimize(ci_md_2, OutputSpec(), calc, steps=2) found_n = 0 @@ -102,7 +102,7 @@ def test_save_results_no_old_props(): found_n += 1 import sys, ase.io #DEBUG ase.io.write(sys.stdout, at, format="extxyz") #DEBUG - assert found_n == 4 * len(list(ci_opt)) + assert found_n == 5 * len(list(ci_opt)) def test_config_type_update(): diff --git a/wfl/calculators/aims.py b/wfl/calculators/aims.py index be40a915..6b0047f6 100644 --- a/wfl/calculators/aims.py +++ b/wfl/calculators/aims.py @@ -48,8 +48,6 @@ class Aims(WFLFileIOCalculator, ASE_Aims): **kwargs: arguments for ase.calculators.aims.Aims See https://wiki.fysik.dtu.dk/ase/_modules/ase/calculators/aims.html. """ - implemented_properties = ["energy", "forces", "stress"] - # new default value of num_inputs_per_python_subprocess for calculators.generic, # to override that function's built-in default of 10 wfl_generic_default_autopara_info = {"num_inputs_per_python_subprocess": 1} diff --git a/wfl/utils/save_calc_results.py b/wfl/utils/save_calc_results.py index 9e1a2e51..e123b306 100644 --- a/wfl/utils/save_calc_results.py +++ b/wfl/utils/save_calc_results.py @@ -1,10 +1,24 @@ import re +import warnings +from ase.outputs import ArrayProperty, all_outputs from ase.calculators.singlepoint import SinglePointCalculator from ase.calculators.calculator import all_properties, PropertyNotImplementedError -per_atom_properties = ['forces', 'stresses', 'charges', 'magmoms', 'energies'] -per_config_properties = ['energy', 'stress', 'dipole', 'magmom', 'free_energy'] + +# Following ase.io.extxyz +# Determine 'per-atom' and 'per-config' based on all_outputs shape, +# but filter for things in all_properties because that's what +# SinglePointCalculator accepts +per_atom_properties = [] +per_config_properties = [] +for key, val in all_outputs.items(): + if key not in all_properties: + continue + if isinstance(val, ArrayProperty) and val.shapespec[0] == 'natoms': + per_atom_properties.append(key) + else: + per_config_properties.append(key) def save_calc_results(atoms, *, prefix, properties): @@ -46,42 +60,35 @@ def save_calc_results(atoms, *, prefix, properties): if prefix + p in atoms.arrays: del atoms.arrays[prefix + p] + # Make note of implemented properties, if applicable + if isinstance(atoms.calc, SinglePointCalculator): + calc_implemented_properties=None + else: + calc_implemented_properties=atoms.calc.implemented_properties + # copy per-config and per-atom results config_results = {} atoms_results = {} - # use Atoms.get_ methods - if 'energy' in properties: - try: - config_results['energy'] = atoms.get_potential_energy(force_consistent=True) - except PropertyNotImplementedError: - config_results['energy'] = atoms.get_potential_energy() - if 'stress' in properties: - # OLD COMMENT: Quantum Espresso doesn't calculate stress, even if asked for, if pbc=False. - # hopefully this is taken care of by generic calculator cleaning up handling of - # nonperiodic cells - config_results['stress'] = atoms.get_stress() - if 'dipole' in properties: - config_results['dipole'] = atoms.get_dipole_moment() - if 'magmom' in properties: - config_results['magmom'] = atoms.get_magnetic_moment() + for prop_name in properties: + # Sometimes a property is in `calc.results`, but not in `calc.implemented_properties` + # and `calc.get_property` fails later. Skip those. + if calc_implemented_properties is not None and prop_name not in calc_implemented_properties: + continue + if prop_name == 'energy': + try: + config_results['energy'] = atoms.get_potential_energy(force_consistent=True) + except PropertyNotImplementedError: + config_results['energy'] = atoms.get_potential_energy() + continue + if prop_name in per_config_properties: + config_results[prop_name] = atoms.calc.get_property(prop_name, allow_calculation=False) + if prop_name in per_atom_properties: + atoms_results[prop_name] = atoms.calc.get_property(prop_name, allow_calculation=False) try: if prefix is not None: config_results['converged'] = atoms.calc.converged except AttributeError as exc: pass - - # copy per-atom results - if 'forces' in properties: - atoms_results['forces'] = atoms.get_forces() - if 'stresses' in properties: - atoms_results['stresses'] = atoms.get_stresses() - if 'charges' in properties: - atoms_results['charges'] = atoms.get_charges() - if 'magmoms' in properties: - atoms_results['magmoms'] = atoms.get_magnetic_moments() - if 'energies' in properties: - atoms_results['energies'] = atoms.get_potential_energies() - if "extra_results" in dir(atoms.calc): if prefix is None and (len(atoms.calc.extra_results.get("config", {})) > 0 or len(atoms.calc.extra_results.get("atoms", {})) > 0):