search

choderalab / modelforge

Infrastructure to implement and train NNPs
https://modelforge.readthedocs.io/en/latest/
MIT License
9 stars 4 forks source link

Ref ani neighborlist #152

Closed wiederm closed 3 weeks ago

wiederm commented 3 weeks ago

Description

This PR makes sure that the ANI model reduces a pairlist/neighborlist with redundant atom pairs (i.e., atom indices for the pair atom 1 and atom 2 are ((1,2),(2,1))). For ANI, we require non-redudant atom pairs. There are multiple points where the different pairlist could be implemented. I opted for: (1) a keyword provided to the dataset generation that controls whether to include redundant pairs or not (2) inside the ANI model an additional indexing step that will reduce a redundant to a non redundant pairlist

Status

chrisiacovella commented 3 weeks ago

So I guess the real issue here is when we are setting up the dataset in the DataModule, we are unaware of the NNP model (which determines whether we require unique pairs).

I think the solution here is probably the easiest and best, as it retains the modularity (output of the DataModule can be passed to any NNP). The other way to handle this would be to pass the model to the DataModule to extra the unique_pairs_parameter (since this is fundamental to the NNP, I don't think we'd want anyone having to set this on their, as that could be problematic).