Add CPP NL converter for PET #289
Conversation
|
I have fixed your issue with spozdn/pet@e6c4a02 |
| @@ -5,6 +5,21 @@ | |||
| from metatensor.torch.atomistic import NeighborListOptions, System | |||
|
|
|||
|
|
|||
| def process_neighbors( | |||
There was a problem hiding this comment.
why do you redefine a function that just calls the operation?
There was a problem hiding this comment.
Not sure 100%. I took this piece of code from the original pet repo, and there was a reason (maybe TorchScript related) to wrap this up. I'll try to remove the wrapper and we'll see how it goes
There was a problem hiding this comment.
the only reason I could see would be to use keyword arguments, but there is a way to do this directly with the C++ operation (requiring some small changes to the function registration)
| return system_dict | ||
|
|
||
|
|
||
| def get_system_batch_dict_old( |
There was a problem hiding this comment.
It is not used, but this is a pure torch implementation of the converter, that (1) contains the code that we can use for benchmarking and (2) contains a lot of comments on how the data is stored in PET. In principle, I can remove it, but I want to at least save the comments somehow.
There was a problem hiding this comment.
you can add a large comment on top of this file explaining the data format!
I checked the code here, and it seems mostly fine. The PET commit you linked looks good as well, so I think the main remaining issue for smooth integration with LAMMPS & friends will be spozdn/pet#17 |
This commit adds a workaround for a PET neighborlist conververter, which takes the NL in the ASE format and converts it to a PET-compatible format, which is required by the PET message-passing subroutines. It also adds a pure-torch implementation of the same conversion for benchmarking purposes.
Contributor (creator of pull-request) checklist
📚 Documentation preview 📚: https://metatrain--289.org.readthedocs.build/en/289/