In the review of #320 there was a short discussion about making pbc a per-system attribute so that systems with different pbc can be mixed in one batch (#320 (comment)).
It was deferred to a separate issue at the time, but I don't think that issue was ever opened, so raising it here.
I think this would be a useful addition. One concern in that discussion was that mixed pbc may complicate the neighbor list calculations (#320 (comment)), but the current neighbor list API already supports per-system pbc (e.g. batch_naive_neighbor_list in nvalchemi-toolkit-ops takes pbc of shape (num_systems, 3): https://github.com/NVIDIA/nvalchemi-toolkit-ops/blob/2b7c3c3adfb1ca84b886eecbf14bc60ff6ba1dc2/nvalchemiops/torch/neighbors/batch_naive.py#L765-L767), and I think other methods support similar APIs, so it can be handled without making the neighbor list logic so complicated.
What do you think? If we agree on a direction I'd be happy to work on a PR.
In the review of #320 there was a short discussion about making
pbca per-system attribute so that systems with different pbc can be mixed in one batch (#320 (comment)).It was deferred to a separate issue at the time, but I don't think that issue was ever opened, so raising it here.
I think this would be a useful addition. One concern in that discussion was that mixed pbc may complicate the neighbor list calculations (#320 (comment)), but the current neighbor list API already supports per-system pbc (e.g.
batch_naive_neighbor_listin nvalchemi-toolkit-ops takespbcof shape(num_systems, 3): https://github.com/NVIDIA/nvalchemi-toolkit-ops/blob/2b7c3c3adfb1ca84b886eecbf14bc60ff6ba1dc2/nvalchemiops/torch/neighbors/batch_naive.py#L765-L767), and I think other methods support similar APIs, so it can be handled without making the neighbor list logic so complicated.What do you think? If we agree on a direction I'd be happy to work on a PR.