Manual charges #14
Manual charges #14
Conversation
…it charge treatment.
There was a problem hiding this comment.
Very nice. I made some minor comments. If you could be able to fix them this would be nice. Besides the linter is not happy:
/home/runner/work/MeshLODE/MeshLODE/src/meshlode/calculators/meshpotential.py:245:13: B007 Loop control variable 'types_single' not used within the loop body. If this is intended, start the name with an underscore.
/home/runner/work/MeshLODE/MeshLODE/tests/calculators/test_meshpotential.py:110:5: B018 Found useless Tuple expression. Consider either assigning it to a variable or removing it.
You can test your changes also locally with tox -e lint (tox -e format does not fix all issues, especially not renaming of code.)
I will do the metatensor version afterwards. As we discussed the idea there is that the key of the block will be [“center_type”, “charge_channel”] if explicit charges are given instead of ["center_type", "neighbor_type"] for one hot charges.
| @@ -95,6 +104,19 @@ def test_single_frame(): | |||
| ) | |||
|
|
|||
|
|
|||
| def test_single_frame_with_charges(): | |||
There was a problem hiding this comment.
Maybe a comment here that this test uses an explicit one hot encoding for the charges.
PicoCentauri
force-pushed
the
manual_charges
branch
from
5dc2a7c to
a77ca44
Compare
| charges_single = self._one_hot_charges( | ||
| types_single, | ||
| requested_types, | ||
| n_types, |
There was a problem hiding this comment.
This is just len(requested_types) as you write in L199 I think you don't need this as a parameter.
| dtype: torch.dtype, | ||
| device: torch.device, |
There was a problem hiding this comment.
Just for convenience and consistence with torch, I would make them optional.
| dtype: torch.dtype, | |
| device: torch.device, | |
| dtype: Optional[torch.dtype]=None, | |
| device: Optional[torch.device]=None, |
| for spec_neighbor in range(len(requested_types)): | ||
| a_pair = spec_center * n_types + spec_neighbor | ||
| for spec_channel in range(len(spec_channels)): | ||
| a_pair = spec_center * n_charges_channels + spec_channel |
📚 Documentation preview 📚: https://meshlode--14.org.readthedocs.build/en/14/