search

jimlutsko / classicalDFT

GNU General Public License v3.0
9 stars 1 forks source link

Cedric fix #31

Closed ceschoon closed 3 years ago

ceschoon commented 3 years ago

I modified the way weights are computed in both the FMT and interaction code. The formulas are still the same but the implementation is a bit different. The old one was incorrect when dx!=dy!=dz.

The issue in the FMT code was that weights were computed by checking whether or not a grid point in in or out the hard sphere radius using a notion of distance based on grid indices rather than actual distances. For example, the hard sphere radius was expressed in number of points on the grid as R=hsr/dx. When dx!=dy!=dz, this does not make sense anymore and points in the y or z direction would be misclassified as in or out the hard sphere radius.

The issue in the interaction code was assuming symmetry under the permutation of x,y,z for the grid points. The integration on the inside of a sphere was thus restricted to a single quadrant (x>0,y>0,z>0 -- reflexion symmetry, ok) as well as z<y<x (not ok). This is incorrect when dx!=dy!=dz because 1) the grid points do not fall at the same positions on the x,y,z axes and 2) there may be more points in the y or z direction, which would be missed assuming z<y<x for the grid indices.

I checked that the result of a homogeneous solid calculation remains identical when dx=dy=dz, before and after my modifications.

Note: I left the old weight generation in Interaction.cpp (name = "generateWeightsXYZSym"). It is still used by default when dx=dy=dz as it is more efficient. (Of course I disabled this when testing the new code I created on the case dx=dy=dz.)

Cedric

jimlutsko commented 3 years ago

OK, I have just started to work on the dynamical stuff. I will begin testing tomorrow and it should go pretty quickly.

jim

On Wed, Jun 2, 2021 at 10:00 AM ceschoon @.***> wrote:

I modified the way weights are computed in both the FMT and interaction code. The formulas are still the same but the implementation is a bit different. The old one was incorrect when dx!=dy!=dz.

The issue in the FMT code was that weights were computed by checking whether or not a grid point in in or out the hard sphere radius using a notion of distance based on grid indices rather than actual distances. For example, the hard sphere radius was expressed in number of points on the grid as R=hsr/dx. When dx!=dy!=dz, this does not make sense anymore and points in the y or z direction would be misclassified as in or out the hard sphere radius.

The issue in the interaction code was assuming symmetry under the permutation of x,y,z for the grid points. The integration on the inside of a sphere was thus restricted to a single quadrant (x>0,y>0,z>0 -- reflexion symmetry, ok) as well as z<y<x (not ok). This is incorrect when dx!=dy!=dz because 1) the grid points do not fall at the same positions on the x,y,z axes and 2) there may be more points in the y or z direction, which would be missed assuming z<y<x for the grid indices.

I checked that the result of a homogeneous solid calculation remains identical when dx=dy=dz, before and after my modifications.

Note: I left the old weight generation in Interaction.cpp (name = "generateWeightsXYZSym"). It is still used by default when dx=dy=dz as it is more efficient. (Of course I disabled this when testing the new code I created on the case dx=dy=dz.)

Cedric

You can view, comment on, or merge this pull request online at:

https://github.com/jimlutsko/classicalDFT/pull/31 Commit Summary

  • FMT weights for dx!=dy!=dz
  • Interaction: correction to generateWeights() to work with dx!=dy!=dz
  • fix to recent change in Interpolation
  • Merge branch 'develop' of https://github.com/jimlutsko/classicalDFT into Cedric_fix
  • 2nd fix to recent change in Interpolation
  • (Minimizer) prevent alpha from becoming too small
  • Revert "(Minimizer) prevent alpha from becoming too small"

File Changes

Patch Links:

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/jimlutsko/classicalDFT/pull/31, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVPPKWATLXLN24J7KWMOZTTQXQKJANCNFSM456HJQCA .

-- % Jim Lutsko, CNLPCS, Universite Libre de Bruxelles, % Campus Plaine -- CP231 B-1050 Bruxelles, Belgium % tel: +32-2-650-5997 email: @.*** % fax: +32-2-650-5767