Fix RDF normalization when type A and B are different #30
Conversation
… of A and B types
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
=======================================
Coverage 99.62% 99.63%
=======================================
Files 5 5
Lines 270 273 +3
=======================================
+ Hits 269 272 +3
Misses 1 1
|
| @@ -22,6 +22,15 @@ def test_gsd_rdf_samename(self, gsdfile_bond): | |||
| assert norm2 == 1 | |||
| assert not np.array_equal(rdf_noex, rdf_ex) | |||
|
|
|||
| def test_gsd_rdf_pair_order(self, gsdfile_bond): | |||
There was a problem hiding this comment.
This unit test is a perfect example of testing that the function does what we want 🙌
| @@ -145,6 +147,8 @@ def gsd_rdf( | |||
| rdf.compute(aq, neighbors=nlist, reset=False) | |||
|
|
|||
| normalization = post_filter / pre_filter if exclude_bonded else 1 | |||
| normalization *= ab_ratio | |||
There was a problem hiding this comment.
LGTM! I'm not sure theory-wise why this fixes it. @erjank Do you have a good explanation? If we're still stumped, we should reach out to Bradley or Vyas to help us understand.
There was a problem hiding this comment.
I think it's something related to the AABB query module, but I'm not that familiar with it. Maybe something related to exclude_ii?
I left a comment on the issue:
I'm not sure if this is related to the post_filter and pre_filter implementation. Those variables are only defined when exclude_bonded=True, and this issue occurs regardless of what is set for exclude_bonded.
I wonder if it is instead due to the the neighbor list query step. system is always created using A_pos and the query is always done for B_pos which probably explains why the behavior depends on what order A_name and B_name are defined.
This addresses #28 and fixes the RDF normalization errors that result when types A and B are different, and there are different amount of each type.
I'm leaving this as a WIP, as I don't think we are totally sure of the origin of the issue and might want to discuss it more, but this does fix the normalization issues. In other words, the RDFs returned are the same regardless of what order types A and B are passed into
gsd_rdfwhen there are different amounts of A/B types in the system..