diff --git a/metatomic-torch/CHANGELOG.md b/metatomic-torch/CHANGELOG.md index 4191ca1c1..62733f346 100644 --- a/metatomic-torch/CHANGELOG.md +++ b/metatomic-torch/CHANGELOG.md @@ -24,6 +24,14 @@ a changelog](https://keepachangelog.com/en/1.1.0/) format. This project follows - 2-argument `unit_conversion_factor(from_unit, to_unit)` that parses arbitrary unit expressions and checks dimensional compatibility +### Deprecated + +- `ModelOutput.quantity` field is deprecated; use the 2-argument `unit()` instead + for unit specification. Unit conversion now determines dimensions from the + expression automatically. +- `TensorMap.set_info("quantity", ...)` convention is deprecated; quantity is + no longer required for unit conversion. + ### Changed - 3-argument `unit_conversion_factor(quantity, from_unit, to_unit)` is diff --git a/metatomic-torch/include/metatomic/torch/model.hpp b/metatomic-torch/include/metatomic/torch/model.hpp index 2a0d516c6..81b11318d 100644 --- a/metatomic-torch/include/metatomic/torch/model.hpp +++ b/metatomic-torch/include/metatomic/torch/model.hpp @@ -55,13 +55,17 @@ class METATOMIC_TORCH_EXPORT ModelOutputHolder: public torch::CustomClassHolder /// description of this output, defaults to empty string of not set by the user std::string description; - /// quantity of the output (e.g. energy, dipole, …). If this is an empty + /// quantity of the output (e.g. energy, dipole, ...). If this is an empty /// string, no unit conversion will be performed. + /// @deprecated This field is no longer required for unit conversion. + /// The unit parser determines dimensions from the expression itself. + [[deprecated("quantity is no longer required for unit conversion, use unit directly")]] const std::string& quantity() const { return quantity_; } /// set the quantity of the output + [[deprecated("quantity is no longer required for unit conversion, use unit directly")]] void set_quantity(std::string quantity); /// unit of the output. If this is an empty string, no unit conversion will diff --git a/metatomic-torch/src/model.cpp b/metatomic-torch/src/model.cpp index ea8c682d2..de37bc430 100644 --- a/metatomic-torch/src/model.cpp +++ b/metatomic-torch/src/model.cpp @@ -54,6 +54,13 @@ static void read_vector_int_json( /******************************************************************************/ void ModelOutputHolder::set_quantity(std::string quantity) { + if (!quantity.empty()) { + TORCH_WARN_DEPRECATION( + "ModelOutput.quantity is deprecated and will be removed in a future version. " + "The quantity field is no longer required for unit conversion since the " + "unit parser determines dimensions from the expression itself." + ); + } if (valid_quantity(quantity)) { validate_unit(quantity, unit_); } diff --git a/metatomic-torch/tests/models.cpp b/metatomic-torch/tests/models.cpp index c319b6593..4ca4528f2 100644 --- a/metatomic-torch/tests/models.cpp +++ b/metatomic-torch/tests/models.cpp @@ -109,11 +109,15 @@ TEST_CASE("Models metadata") { struct WarningHandler: public torch::WarningHandler { virtual ~WarningHandler() override = default; void process(const torch::Warning& warning) override { - auto expected = std::string( - "unknown quantity 'unknown', only [charge energy force heat_flux " - "length mass momentum pressure velocity] are supported" - ); - CHECK(warning.msg() == expected); + // First warning is deprecation, second is unknown quantity + // We check for the unknown quantity warning + if (warning.msg().find("unknown quantity") != std::string::npos) { + auto expected = std::string( + "unknown quantity 'unknown', only [charge energy force heat_flux " + "length mass momentum pressure velocity] are supported" + ); + CHECK(warning.msg() == expected); + } } }; diff --git a/python/metatomic_ase/pyproject.toml b/python/metatomic_ase/pyproject.toml index a27cc385c..faf71e8a7 100644 --- a/python/metatomic_ase/pyproject.toml +++ b/python/metatomic_ase/pyproject.toml @@ -44,6 +44,8 @@ python_files = ["*.py"] testpaths = ["tests"] filterwarnings = [ "error", + # ModelOutput.quantity deprecation (C++ TORCH_WARN_DEPRECATION) + "ignore:ModelOutput.quantity is deprecated:DeprecationWarning", # TorchScript deprecation warnings "ignore:`torch.jit.script` is deprecated. Please switch to `torch.compile` or `torch.export`:DeprecationWarning", "ignore:`torch.jit.script_method` is deprecated. Please switch to `torch.compile` or `torch.export`:DeprecationWarning", diff --git a/python/metatomic_ase/tests/calculator.py b/python/metatomic_ase/tests/calculator.py index ac85d598f..7dbd25bf7 100644 --- a/python/metatomic_ase/tests/calculator.py +++ b/python/metatomic_ase/tests/calculator.py @@ -848,7 +848,8 @@ def test_additional_input(atoms): assert head == "extra" assert name in inputs - assert tensor.get_info("quantity") == inputs[name].quantity + # quantity info is no longer set (deprecated); just check unit is set + assert tensor.get_info("unit") == inputs[name].unit values = tensor[0].values.numpy() expected = ARRAY_QUANTITIES[name]["getter"](atoms).reshape(values.shape) diff --git a/python/metatomic_ase/tests/conftest.py b/python/metatomic_ase/tests/conftest.py index 2b9201ed0..da4d7423e 100644 --- a/python/metatomic_ase/tests/conftest.py +++ b/python/metatomic_ase/tests/conftest.py @@ -1,10 +1,22 @@ +import re + import pytest +# C++ TORCH_WARN_DEPRECATION writes to stderr; filter the known quantity warning. +# Torch 2.3 emits "[W model.cpp:N] ..." (no timestamp); Torch 2.11+ emits +# "[W HH:MM:SS.fractZ model.cpp:N] ...". The [^\]]* tolerates both. +_QUANTITY_DEPRECATION_RE = re.compile( + r"\[W[^\]]*model\.cpp:\d+\] Warning: ModelOutput\.quantity is " + r"deprecated.*\(function set_quantity\)\n" +) + + @pytest.fixture(autouse=True) def fail_test_with_output(capfd): yield captured = capfd.readouterr() # the code should not print anything to stdout or stderr assert captured.out == "" - assert captured.err == "" + err = _QUANTITY_DEPRECATION_RE.sub("", captured.err) + assert err == "" diff --git a/python/metatomic_torch/metatomic/torch/documentation.py b/python/metatomic_torch/metatomic/torch/documentation.py index 5dc080c05..49c10b6c2 100644 --- a/python/metatomic_torch/metatomic/torch/documentation.py +++ b/python/metatomic_torch/metatomic/torch/documentation.py @@ -295,6 +295,11 @@ def quantity(self) -> str: Quantity of the output (e.g. energy, dipole, …). If this is an empty string, no unit conversion will be performed. + .. deprecated:: + The ``quantity`` field is deprecated and will be removed. + Unit conversion determines dimensions from the unit expression. + Set ``quantity`` to an empty string to suppress deprecation warnings. + The list of possible quantities is available :ref:`here `. """ diff --git a/python/metatomic_torch/metatomic/torch/model.py b/python/metatomic_torch/metatomic/torch/model.py index 22f83c0cd..566fbf4b6 100644 --- a/python/metatomic_torch/metatomic/torch/model.py +++ b/python/metatomic_torch/metatomic/torch/model.py @@ -265,7 +265,6 @@ class AtomisticModel(torch.nn.Module): >>> capabilities = ModelCapabilities( ... outputs={ ... "energy": ModelOutput( - ... quantity="energy", ... unit="eV", ... per_atom=False, ... explicit_gradients=[], @@ -507,16 +506,9 @@ def forward( for name, output in outputs.items(): declared = self._capabilities.outputs[name] requested = options.outputs.get(name, ModelOutput()) - if declared.quantity == "" or requested.quantity == "": + if declared.unit == "" or requested.unit == "": continue - if declared.quantity != requested.quantity: - raise ValueError( - f"model produces values as '{declared.quantity}' for the " - f"'{name}' output, but the engine requested " - f"'{requested.quantity}'" - ) - conversion = unit_conversion_factor( declared.unit, requested.unit, @@ -994,7 +986,9 @@ def _convert_systems_units( tensor = system.get_data(name) unit = tensor.get_info("unit") - if requested.quantity != "" and unit is not None: + # Convert units if both the tensor and requested output have units. + # The quantity field is deprecated; unit conversion is dimension-aware. + if unit is not None and requested.unit != "": conversion = unit_conversion_factor( unit, requested.unit, @@ -1032,7 +1026,6 @@ def _convert_systems_units( blocks=new_blocks, ) new_tensor.set_info("unit", requested.unit) - new_tensor.set_info("quantity", requested.quantity) new_system.add_data(name, new_tensor) new_systems.append(new_system) diff --git a/python/metatomic_torch/pyproject.toml b/python/metatomic_torch/pyproject.toml index 249b7def1..7e2f395c0 100644 --- a/python/metatomic_torch/pyproject.toml +++ b/python/metatomic_torch/pyproject.toml @@ -58,6 +58,8 @@ python_files = ["*.py"] testpaths = ["tests"] filterwarnings = [ "error", + # ModelOutput.quantity deprecation + "ignore:ModelOutput.quantity is deprecated:DeprecationWarning", # TorchScript deprecation warnings "ignore:`torch.jit.script` is deprecated. Please switch to `torch.compile` or `torch.export`:DeprecationWarning", "ignore:`torch.jit.save` is deprecated. Please switch to `torch.export`:DeprecationWarning", diff --git a/python/metatomic_torch/tests/conftest.py b/python/metatomic_torch/tests/conftest.py index 2b9201ed0..da4d7423e 100644 --- a/python/metatomic_torch/tests/conftest.py +++ b/python/metatomic_torch/tests/conftest.py @@ -1,10 +1,22 @@ +import re + import pytest +# C++ TORCH_WARN_DEPRECATION writes to stderr; filter the known quantity warning. +# Torch 2.3 emits "[W model.cpp:N] ..." (no timestamp); Torch 2.11+ emits +# "[W HH:MM:SS.fractZ model.cpp:N] ...". The [^\]]* tolerates both. +_QUANTITY_DEPRECATION_RE = re.compile( + r"\[W[^\]]*model\.cpp:\d+\] Warning: ModelOutput\.quantity is " + r"deprecated.*\(function set_quantity\)\n" +) + + @pytest.fixture(autouse=True) def fail_test_with_output(capfd): yield captured = capfd.readouterr() # the code should not print anything to stdout or stderr assert captured.out == "" - assert captured.err == "" + err = _QUANTITY_DEPRECATION_RE.sub("", captured.err) + assert err == "" diff --git a/python/metatomic_torch/tests/examples.py b/python/metatomic_torch/tests/examples.py index d846f0f07..5aadd9d24 100644 --- a/python/metatomic_torch/tests/examples.py +++ b/python/metatomic_torch/tests/examples.py @@ -47,7 +47,7 @@ def test_export_atomistic_model(tmp_path): ) outputs = { - "energy": ModelOutput(quantity="energy", unit="eV", per_atom=False), + "energy": ModelOutput(unit="eV", per_atom=False), } # run bare model diff --git a/python/metatomic_torch/tests/model.py b/python/metatomic_torch/tests/model.py index 3937ecd11..22769212c 100644 --- a/python/metatomic_torch/tests/model.py +++ b/python/metatomic_torch/tests/model.py @@ -702,7 +702,6 @@ def test_not_requested_output(system): def test_systems_unit_conversion(system): requested_inputs = { "masses": ModelOutput( - quantity="mass", unit="kg", per_atom=True, ), diff --git a/python/metatomic_torch/tests/units.py b/python/metatomic_torch/tests/units.py index 95d6ffec9..e288f6c88 100644 --- a/python/metatomic_torch/tests/units.py +++ b/python/metatomic_torch/tests/units.py @@ -212,41 +212,42 @@ def test_overflow_division(): def test_valid_units(): # just checking that all of these are valid - ModelOutput(quantity="length", unit="A") - ModelOutput(quantity="length", unit="Angstrom") - ModelOutput(quantity="length", unit="Bohr") - ModelOutput(quantity="length", unit="meter") - ModelOutput(quantity="length", unit=" centimeter") - ModelOutput(quantity="length", unit="cm") - ModelOutput(quantity="length", unit="millimeter") - ModelOutput(quantity="length", unit="mm") - ModelOutput(quantity="length", unit=" micrometer") - ModelOutput(quantity="length", unit="um") - ModelOutput(quantity="length", unit="µm") - ModelOutput(quantity="length", unit="nanometer") - ModelOutput(quantity="length", unit="nm ") - - ModelOutput(quantity="energy", unit="eV") - ModelOutput(quantity="energy", unit="meV") - ModelOutput(quantity="energy", unit="Hartree") - ModelOutput(quantity="energy", unit="kcal / mol ") - ModelOutput(quantity="energy", unit="kJ/mol") - ModelOutput(quantity="energy", unit="Joule") - ModelOutput(quantity="energy", unit="J") - ModelOutput(quantity="energy", unit="Rydberg") - ModelOutput(quantity="energy", unit="Ry") - - ModelOutput(quantity="force", unit="eV/Angstrom") - ModelOutput(quantity="force", unit="eV/A") - - ModelOutput(quantity="pressure", unit="eV/Angstrom^3") - ModelOutput(quantity="pressure", unit="eV/A^3") - - ModelOutput(quantity="momentum", unit="u * A/ fs") - ModelOutput(quantity="momentum", unit=" (eV*u )^(1/ 2 )") - - ModelOutput(quantity="velocity", unit="A/fs") - ModelOutput(quantity="velocity", unit="A/s") + # quantity parameter is deprecated, only testing unit parsing + ModelOutput(unit="A") + ModelOutput(unit="Angstrom") + ModelOutput(unit="Bohr") + ModelOutput(unit="meter") + ModelOutput(unit=" centimeter") + ModelOutput(unit="cm") + ModelOutput(unit="millimeter") + ModelOutput(unit="mm") + ModelOutput(unit=" micrometer") + ModelOutput(unit="um") + ModelOutput(unit="µm") + ModelOutput(unit="nanometer") + ModelOutput(unit="nm ") + + ModelOutput(unit="eV") + ModelOutput(unit="meV") + ModelOutput(unit="Hartree") + ModelOutput(unit="kcal / mol ") + ModelOutput(unit="kJ/mol") + ModelOutput(unit="Joule") + ModelOutput(unit="J") + ModelOutput(unit="Rydberg") + ModelOutput(unit="Ry") + + ModelOutput(unit="eV/Angstrom") + ModelOutput(unit="eV/A") + + ModelOutput(unit="eV/Angstrom^3") + ModelOutput(unit="eV/A^3") + + ModelOutput(unit="u * A/ fs") + ModelOutput(unit=" (eV*u )^(1/ 2 )") + + ModelOutput(unit="A/fs") + ModelOutput(unit="A/s") # ---- Accumulated floating-point error with fractional powers ---- @@ -318,6 +319,7 @@ def test_micro_sign_microsecond(): def test_quantity_unit_mismatch(): # energy quantity with force unit + # quantity parameter is deprecated, but we still test unit validation with pytest.raises((ValueError, RuntimeError), match="incompatible with quantity"): ModelOutput(quantity="energy", unit="eV/A") diff --git a/python/metatomic_torchsim/pyproject.toml b/python/metatomic_torchsim/pyproject.toml index 9c3b119ef..5624a9841 100644 --- a/python/metatomic_torchsim/pyproject.toml +++ b/python/metatomic_torchsim/pyproject.toml @@ -45,6 +45,8 @@ python_files = ["*.py"] testpaths = ["tests"] filterwarnings = [ "error", + # ModelOutput.quantity deprecation (C++ TORCH_WARN_DEPRECATION) + "ignore:ModelOutput.quantity is deprecated:DeprecationWarning", # TorchScript deprecation warnings "ignore:`torch.jit.script` is deprecated. Please switch to `torch.compile` or `torch.export`:DeprecationWarning", "ignore:`torch.jit.script_method` is deprecated. Please switch to `torch.compile` or `torch.export`:DeprecationWarning", diff --git a/python/metatomic_torchsim/tests/conftest.py b/python/metatomic_torchsim/tests/conftest.py index 2b9201ed0..da4d7423e 100644 --- a/python/metatomic_torchsim/tests/conftest.py +++ b/python/metatomic_torchsim/tests/conftest.py @@ -1,10 +1,22 @@ +import re + import pytest +# C++ TORCH_WARN_DEPRECATION writes to stderr; filter the known quantity warning. +# Torch 2.3 emits "[W model.cpp:N] ..." (no timestamp); Torch 2.11+ emits +# "[W HH:MM:SS.fractZ model.cpp:N] ...". The [^\]]* tolerates both. +_QUANTITY_DEPRECATION_RE = re.compile( + r"\[W[^\]]*model\.cpp:\d+\] Warning: ModelOutput\.quantity is " + r"deprecated.*\(function set_quantity\)\n" +) + + @pytest.fixture(autouse=True) def fail_test_with_output(capfd): yield captured = capfd.readouterr() # the code should not print anything to stdout or stderr assert captured.out == "" - assert captured.err == "" + err = _QUANTITY_DEPRECATION_RE.sub("", captured.err) + assert err == ""