Skip to content

Added sample_kind to ModelOutput, without breaking changes#168

Open
pfebrer wants to merge 5 commits intomainfrom
sample_kind
Open

Added sample_kind to ModelOutput, without breaking changes#168
pfebrer wants to merge 5 commits intomainfrom
sample_kind

Conversation

@pfebrer
Copy link
Copy Markdown

@pfebrer pfebrer commented Feb 26, 2026 •
edited by github-actions bot
Loading

This PR substitutes #165.

It also implements sample_kind, with the short-term goal of enabling per-pair targets, but in this case in a way that doesn't break backward compatibility (new code using old ModelOutput might fail, but old code using the new ModelOutput won't). This is achieved by keeping the possibility to pass per_atom as an argument, and setting/getting it as a property, even though from now on the only thing ModelOutput stores is sample_kind.

Thanks to @Luthaf for letting me know that one can set optional arguments in torch exportable classes :)

If you agree with merging this one I will finalize it with some more tests ✌️

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

📚 Download documentation for this pull-request

Luthaf
Luthaf reviewed Feb 27, 2026
View reviewed changes
Comment thread metatomic-torch/include/metatomic/torch/model.hpp Outdated
Comment thread metatomic-torch/src/model.cpp Outdated
Comment thread metatomic-torch/src/outputs.cpp Outdated
@pfebrer pfebrer force-pushed the sample_kind branch 3 times, most recently from 4b3c32d to 87dc377 Compare March 13, 2026 21:37
@pfebrer
Copy link
Copy Markdown
Author

pfebrer commented Mar 13, 2026

This is ready for a final look. I checked that one can take the conda lammps-metatomic and use this modified metatomic version without problems to run an MD with a pet-mad.pt exported using the last metatomic release. The results of the MD are exactly the same as when running without the modified version.

Luthaf
Luthaf reviewed Mar 16, 2026
View reviewed changes
Comment thread metatomic-torch/include/metatomic/torch/model.hpp
Comment thread metatomic-torch/include/metatomic/torch/model.hpp Outdated
Comment thread metatomic-torch/include/metatomic/torch/model.hpp Outdated
Comment thread metatomic-torch/src/model.cpp Outdated
Comment thread metatomic-torch/src/model.cpp Outdated
Comment thread metatomic-torch/src/outputs.cpp Outdated
Comment thread metatomic-torch/src/outputs.cpp Outdated
@pfebrer pfebrer mentioned this pull request Mar 17, 2026
Open
@Luthaf Luthaf force-pushed the sample_kind branch 4 times, most recently from dff557b to ea5e7fd Compare March 25, 2026 16:50
@Luthaf
Copy link
Copy Markdown
Member

Luthaf commented Mar 25, 2026

Do not merge before #186, we need to have at least one release of metatomic-ase that works with an already released version of metatomic-torch

@Luthaf Luthaf force-pushed the sample_kind branch 3 times, most recently from 9353d37 to 4df02e3 Compare April 8, 2026 08:42
@pfebrer @Luthaf
Change per_atom into sample_kind while keeping backward compatibi…
fca4e05 
…lity

Co-Authored-By: Guillaume Fraux <guillaume.fraux@epfl.ch>
HaoZeke added 4 commits April 17, 2026 01:18
@HaoZeke
deprecate(ModelOutput): tolerate per_atom deprecation warnings in tes…
0c64b28 
…ts and migrate call sites

Mirrors the morfixes pattern for `ModelOutput.quantity`: keep the runtime
`TORCH_WARN_DEPRECATION` on the C++ `per_atom` getter/setter, filter it
out of test stderr via regex, ignore the matching Python
`DeprecationWarning` in pyproject filterwarnings, and migrate internal
callers to `sample_kind`.

Python (metatomic-torch)
- Migrate `_get_requested_inputs` dedup check from `per_atom` to
  `sample_kind` so the internal path no longer trips its own deprecation.
- Add a `.. deprecated::` block to the `per_atom` property docstring in
  documentation.py.

Tests / packaging
- conftest.py for all three subpackages: regex-strip the C++
  `TORCH_WARN_DEPRECATION` line for `per_atom` from stderr in the autouse
  fail-on-output fixture, tolerating both Torch 2.3 (`[W model.cpp:N]`)
  and Torch 2.11+ (`[W<MMDD> HH:MM:SS.fractZ model.cpp:N]`) prefixes. Any
  other unexpected stderr still fails the test.
- `pyproject.toml` for all three subpackages: ignore the matching Python
  `DeprecationWarning` so tests exercising the field directly do not blow
  up under `-W error`.
- Migrate `ModelOutput(per_atom=...)` constructions in `heat_flux.py`,
  `tests/heat_flux.py`, and `tests/calculator.py` to `sample_kind=...`.

Changelog: add a Deprecated entry for `ModelOutput.per_atom`.
@HaoZeke
fix(tests): ruff-format and drop stderr assertions in test_sample_kind
1452667 
Follow-ups to the per_atom deprecation commit:
- `python/metatomic_ase/tests/calculator.py`: collapse the `velocities`
  ModelOutput constructor to a single line to satisfy `ruff format`.
- `python/metatomic_torch/tests/outputs.py`: drop the
  `assert message in captured.err` checks. With the new Python-layer
  `ignore:`per_atom`...:DeprecationWarning` filter, TORCH_WARN routes
  through `PyErr_WarnEx` and is silently dropped, so stderr stays
  empty. The `pytest.warns` checks still verify that the Python
  `DeprecationWarning` is emitted.
@HaoZeke
fix(tests): drop mid-test capfd assertions in test_sample_kind
7fe2a84 
Torch 2.3 writes the C++ `TORCH_WARN_DEPRECATION` directly to stderr
even when the Python-layer `DeprecationWarning` filter is `ignore`,
while Torch 2.11 routes entirely through `PyErr_WarnEx` and leaves
stderr empty. Asserting `captured.err == ""` mid-test fails on Torch
2.3.

Delete the mid-test `capfd.readouterr()` checks and rely on the autouse
`fail_test_with_output` fixture in `conftest.py`, which already
regex-strips the per_atom warning from stderr at end-of-test across
both torch versions. The `pytest.warns(...)` assertions still verify
that each getter/setter emits a Python `DeprecationWarning`.
@HaoZeke
test(ase/heat_flux): migrate lone per_atom=False to sample_kind="system"
72f1fec 
The test previously triggered the per_atom deprecation path through its
ModelOutput construction. Rewriting to sample_kind keeps the test on
the non-deprecated API, so nothing in the test codebase still relies on
the per_atom shim (outside tests that specifically exercise it).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Luthaf Luthaf Luthaf left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pfebrer @Luthaf @HaoZeke