Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong rdf values from InterRDF_s when given 'density=True' #2811

Closed
VOD555 opened this issue Jul 1, 2020 · 0 comments · Fixed by #2812
Closed

Wrong rdf values from InterRDF_s when given 'density=True' #2811

VOD555 opened this issue Jul 1, 2020 · 0 comments · Fixed by #2812

Comments

@VOD555
Copy link
Contributor

VOD555 commented Jul 1, 2020

Expected behavior

If we set density=True, the results from rdf.InterRDF_s should be the same as rdf.InterRDF when given a single-atom to single-atom pair.

See also MDAnalysis/pmda#120

Actual behavior

The results from rdf.InterRDF_s will be 1/N of rdf.InterRDF.
Where N = nA * nB, and nA, nB are the number of atoms in atomgroup A and atomgroup B.

Code to reproduce the behavior

import MDAnalysis as mda
from MDAnalysisTests.datafiles import GRO_MEMPROT, XTC_MEMPROT
from pmda.rdf import InterRDF_s
from MDAnalysis.analysis import rdf

u = mda.Universe(GRO_MEMPROT, XTC_MEMPROT)

s1 = u.select_atoms('name ZND and resid 289')
s2 = u.select_atoms('(name OD1 or name OD2) and resid 51 and sphzone 5.0
                         (resid 289)')
s3 = u.select_atoms('name ZND and (resid 291 or resid 292)')
s4 = u.select_atoms('(name OD1 or name OD2) and sphzone 5.0 (resid 291)')
ags = [[s1, s2], [s3, s4]]

rdfs = InterRDF_s(u, ags, density=True)
rdfs.run()


ref_s1 = u.select_atoms('name ZND and resid 289')
ref_s2 = u.select_atoms('name OD1 and resid 51 and sphzone 5.0 (resid 289)')
ref_rdf = rdf.InterRDF(ref_s1, ref_s2, density=True)
ref_rdf.run()

from numpy.testing import assert_almost_equal

assert_almost_equal(rdf_ref.rdf, rdfs[0][0][0])

Current version of MDAnalysis

  • Which version are you using?
    MDAnalysis1.0.0
  • Which version of Python?
    python 3.6
  • Which operating system?
    Ubuntu 18.04
@orbeckst orbeckst added this to the 1.0.x milestone Jul 1, 2020
orbeckst pushed a commit to VOD555/mdanalysis that referenced this issue Jul 10, 2020
- fix MDAnalysis#2811
  (see also MDAnalysis/pmda#120)
- add test to compare InterRDF and InterRDF_s
- add description for density option (was not documented since 0.19.0)
- updated docs for rdf
  - updated docs with equationi
  - attributes for InterRDF and InterRDF_s
  - reference equations
  - minor grammar/style fixes
  - remove ill-advised usage of InterRDF_s from docs
- update CHANGELOG
orbeckst added a commit that referenced this issue Jul 12, 2020
* Fixes #2811
* enable hitherto undocumented kwarg density for InterRDF_s (but changed 
   meaning of True/False):
   If you used density=False since 0.19.0 to calculate a single particle density then you 
   now have to use density=True. The default remains the same: calculate the RDF and not
   the density.
* Fix normalization for InterRDF_s(..., density=True)
* add test to compare InterRDF and InterRDF_s
* add description for density option
* update CHANGELOG
orbeckst pushed a commit that referenced this issue Jul 12, 2020
- fix #2811
  (see also MDAnalysis/pmda#120)
- add test to compare InterRDF and InterRDF_s
- add description for density option (was not documented since 0.19.0)
- updated docs for rdf
  - updated docs with equationi
  - attributes for InterRDF and InterRDF_s
  - reference equations
  - minor grammar/style fixes
  - remove ill-advised usage of InterRDF_s from docs
- update CHANGELOG
orbeckst added a commit that referenced this issue Jul 12, 2020
- fix (because it was not documented before)
- density=True now means "calculate the single site density";
  default is still "calculate RDF" (now meaning density=False)
- update docs
- update tests (add test for default behavior)
- update CHANGELOG
- backported issue #2811 fix (directly rebased onto master
  instead of added to PR #2798
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this issue Mar 30, 2021
- fix MDAnalysis#2811
  (see also MDAnalysis/pmda#120)
- add test to compare InterRDF and InterRDF_s
- add description for density option (was not documented since 0.19.0)
- updated docs for rdf
  - updated docs with equationi
  - attributes for InterRDF and InterRDF_s
  - reference equations
  - minor grammar/style fixes
  - remove ill-advised usage of InterRDF_s from docs
- update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants