Add an O(3)-symmetrized atomistic model wrapper - #294
Closed
MichelangeloDomina wants to merge 18 commits into
Closed
Add an O(3)-symmetrized atomistic model wrapper#294MichelangeloDomina wants to merge 18 commits into
MichelangeloDomina wants to merge 18 commits into
Conversation
ppegolo
force-pushed
the
review/symmetrized-model-progressive
branch
from
July 28, 2026 09:06
c2acacf to
7e23329
Compare
ppegolo
marked this pull request as ready for review
July 28, 2026 12:42
Luthaf
reviewed
Jul 29, 2026
Luthaf
left a comment
Luthaf
left a comment
Member
There was a problem hiding this comment.
I only checked the code, I'll try to do a pass on the tests ASAP
Comment on lines
+13
to
+33
| Output requests | ||
| --------------- | ||
|
|
||
| The requested output name selects both the source output and the calculation: | ||
|
|
||
| .. list-table:: | ||
| :header-rows: 1 | ||
|
|
||
| * - Requested and returned name | ||
| - Result | ||
| * - ``<name>`` | ||
| - O(3) average of the underlying ``<name>`` output | ||
| * - ``o3::variance::<name>`` | ||
| - component-averaged equivariance variance of ``<name>`` | ||
| * - ``o3::character_projection::<name>`` | ||
| - unnormalized squared character-projection contributions of ``<name>`` | ||
|
|
||
| ``<name>`` is preserved verbatim. It can therefore be a standard quantity, a | ||
| variant such as ``energy/pbe``, or a custom name such as | ||
| ``mtt::feature::node``. For example, | ||
| ``o3::variance::energy/pbe`` evaluates the underlying ``energy/pbe`` output. |
Member
There was a problem hiding this comment.
This is not very clear, but I can rephrase when everything else is done here =)
ppegolo
force-pushed
the
review/symmetrized-model-progressive
branch
from
July 30, 2026 11:26
f551943 to
8f87505
Compare
Luthaf
reviewed
Aug 5, 2026
Luthaf
left a comment
Luthaf
left a comment
Member
There was a problem hiding this comment.
Looks good overall, I mostly have small nits comments.
The one larger thing is that we have two copies of the O3 transformation code, we should only have one.
I only looked at the code, second review for the test coming after that!
Luthaf
reviewed
Aug 5, 2026
Luthaf
left a comment
Luthaf
left a comment
Member
There was a problem hiding this comment.
A lot of tests are explicitly testing the private API. I would rather not (it makes changing the implementation a lot harder) unless there is a good reason for it.
We can also likely reduce the number of tests by re-using the O3 transformation code & tests.
metatrain does not rename deprecated target names on checkpoint upgrade: models keep answering to their native (plural) spellings, and metatomic's AtomisticModel is the alias bridge for engines. Share that bridge's table and name normalization in _quantities.py and use it when categorizing outputs for decomposition, so wrapped models trained with plural names still get per-irrep diagnostics.
O3Transformation.transform_systems and .transform_tensormap cover the same functionality; keeping both meant a second validation layer and a second entry point for every operation. transform_tensormap now accepts integer system_ids of any dtype, converting to long internally.
4 tasks
Member
|
merged in #300 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
metatomic.torch.symmetrized_model: wrap an exportedAtomisticModeltoaverage its predictions over O(3) with a finite rotation quadrature, and to
measure how far the underlying model is from being equivariant.
Everything is selected through regular output requests:
<name>o3::variance::<name>o3::character_projection::<name>SymmetrizedModel.wrap()returns a regular exportableAtomisticModel(metadata, capabilities, neighbor lists, and custom inputs are carried over),
so the wrapped model can be used anywhere the original could, including
TorchScript export.
The conventions (quadrature, variance definition, irrep bookkeeping in
TensorMap keys) are documented in
docs/src/torch/reference/symmetrized-model.rst.Limitations
should be checked by increasing
max_o3_lambda_grid;returned values works).
Contributor (creator of pull-request) checklist
[ ] Issue referenced (for PRs that solve an issue)?Reviewer checklist