Closed rmatsum836 closed 3 years ago
It looks good to me. The only thing I can think of is: should the default behavior of compute_van_hove
have detection built in such that it calls compute_partial_van_hove
with self_correlation = False
for polyatomic pairs and self_correlation = User_Input (default: True)
for monoatomic pairs? Otherwise, every system with more than one atom type will print a warning on the call to compute_van_hove
.
Sounds good, something like?
for elem1, elem2 in it.combinations(...):
if elem1 != elem2:
compute_partial_van_hove(self_correlation=False)
else:
compute_partial_van_hove(self_correlation=True)
Yep! Might be easier to have a boolean variable and just reference that in the existing code.
for elem1, elem2 in it.combinations(...):
myBool = self_correlation # User Input
if elem1 != elem2:
myBool = False
data.append(
[
...
self_correlation = myBool,
...
]
)
Thanks for the suggestion, let me know what you think!
Look's good!
For non-monatomic pairs (oxygen-hydrogen), we shouldn't calculate any self-interactions even if
self_correlation=True
. This PR adds a check tocompute_partial_van_hove
which setsself_correlation
to False ifselection1 != selection2
. User is warned if this happens.Test case has also been added to check that warning is properly raised.