Add example TensorMap layout to TargetInfo
#370
Conversation
frostedoyster
force-pushed
the
targetinfo-tensormap
branch
from
f422470
to
80e0d50
Compare
frostedoyster
force-pushed
the
targetinfo-tensormap
branch
from
80e0d50
to
b8cd9ba
Compare
Luthaf
changed the title
TensorMap to TargetInfoTensorMap to TargetInfo
frostedoyster
force-pushed
the
targetinfo-tensormap
branch
from
7e0c5a3
to
f881951
Compare
pyproject.toml
Outdated
| @@ -68,7 +68,7 @@ pet = [ | |||
| "pet @ git+https://github.com/lab-cosmo/pet@7eddb2e", | |||
| ] | |||
| gap = [ | |||
| "rascaline-torch @ git+https://github.com/luthaf/rascaline@d181b28#subdirectory=python/rascaline-torch", | |||
| "rascaline-torch @ git+https://github.com/luthaf/rascaline@5326b6e#subdirectory=python/rascaline-torch", | |||
There was a problem hiding this comment.
I don't understand why these changes appear here, when they should already be on master. Could you squash this PR & rebase?
There was a problem hiding this comment.
I merged (saved me some time compared to rebasing). This should be gone
src/metatrain/cli/train.py
Outdated
| raise ValueError( | ||
| f"Target information for key {key} differs between training sets." |
There was a problem hiding this comment.
it would be nicer to add what the difference is here, so it is clear to the users
src/metatrain/utils/data/dataset.py
Outdated
| # one of these will be set to True inside the _check_layout method | ||
| self.is_scalar = False | ||
| self.is_cartesian = False | ||
| self.is_spherical = False |
There was a problem hiding this comment.
Should these be @property? This way they can not be modified from the outside
| components=[], | ||
| properties=Labels.range("energy", 1), | ||
| ) | ||
| energy_layout = TensorMap( |
There was a problem hiding this comment.
Please add documentation to these. You can add a doctring for variable with
blabla = Something(
...
)
"""Some doc string here"""And then render them with sphinx.
There was a problem hiding this comment.
I tried but I get the following:
File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
File "/home/filippo/code/metatrain/.tox/docs/lib/python3.12/site-packages/metatrain/utils/testing.py", line 40, in <module>
block_with_position_gradients.add_gradient("positions", position_gradient_block)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'add_gradient'
i.e. these seem to be "documentation" objects. SInce the testing module will already be gone in #376, can we do without the docs?
Luthaf
changed the title
TensorMap to TargetInfoTensorMap layout to TargetInfo
|
Regarding the model checks: supporting generic targets is quite a major efforts so I decided to split it into 4 PRs (target info, readers, models, utilities). The model checks will be part of the third PR (I didn't think it was very smart to put in some code that I would have to remove soon after). The current errors are not terrible |
OK, would have been nice to mention this in the first PR comment, it makes it easier to understand what you are doing! |
Part of #364
Contributor (creator of pull-request) checklist
📚 Documentation preview 📚: https://metatrain--370.org.readthedocs.build/en/370/