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

Test dipolar direct sum methods #4046

Closed
wants to merge 6 commits into from
Closed

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Dec 16, 2020

Partial fix for #2071

Description of changes:

  • add test cases for dds without replica on CPU/GPU, with replica, with DLC

@jngrad
Copy link
Member Author

jngrad commented Dec 16, 2020

offline discussion: the test doesn't pass anymore if the dipoles are relaxed with steepest descent:

diff --git a/testsuite/python/dipolar_mdlc_dds_scafacos_direct.py b/testsuite/python/dipolar_mdlc_dds_scafacos_direct.py
index 7ddcb6c65..0a470baff 100644
--- a/testsuite/python/dipolar_mdlc_dds_scafacos_direct.py
+++ b/testsuite/python/dipolar_mdlc_dds_scafacos_direct.py
@@ -157,6 +157,17 @@ class dds(ut.TestCase):
             system.integrator.set_steepest_descent(
                 f_max=1, gamma=0.001, max_displacement=0.01)
             system.integrator.run(100)
+            dds_cpu = espressomd.magnetostatics.DipolarDirectSumCpu(
+                prefactor=1.2)
+            system.actors.add(dds_cpu)
+            system.integrator.set_steepest_descent(
+                f_max=5,
+                gamma=0.001,
+                max_displacement=0.01)
+            print(system.analysis.energy()["dipolar"])
+            print(system.integrator.run(100))
+            print(system.analysis.energy()["dipolar"])
+            system.actors.clear()
             system.integrator.set_vv()
 
             # compute forces and energies for dawaanr

@RudolfWeeber
Copy link
Contributor

The difference between Espresso's and Scafacos' direct sum with replica seems to be that Espresso uses a spherical cutoff, i.e.
sqrt(i_x^2 +i_y^2 +i_z^2) <= n_replica^2
whereas scafacos does the full cube.
We probably better check, whether the dds with replica gets monotonically closer to p3m with increasing number of replica or compare or a tiny system against results calculate din Python.

@jngrad
Copy link
Member Author

jngrad commented Dec 17, 2020

I tried with 2 to 8 replica, with and without DLC, but the deviation to ScaFaCoS did not decrease, while the runtime increased to the order of 1 min.

@RudolfWeeber
Copy link
Contributor

It is not possible to make Espressos direct sum with replica agree to scafacos direct sum with replica.
They use different cutoff rules:

  • Espresso: Spherical cutoff on the replica lavel. Only include boxes when i_x^2+i_y^2+i_z^2 <=n_replica^2, ix,y,z in N
  • Scafacos: Either don't use spherical cutoff at all (i.e., n_replica=4,4,4 also cinludes i_x=-4,i_y=-4, i_z=-4, or cutoff on the particle distance level (using the cutoff argument).

I'm therefore removing this test.

The DDS with replica method doesn't agree with the ScaFaCoS direct
method because a different cutoff scheme is used.
@RudolfWeeber
Copy link
Contributor

Dipolar direct sum with replica is broken for 0 replica. In that case, it should match with the direct summation for open boundaries, which it doesn't
dds_cpu without replica, dds from scafacos, dds gpu match

@jngrad
Copy link
Member Author

jngrad commented Dec 17, 2020

Dipolar direct sum with replica is broken for 0 replica. In that case, it should match with the direct summation for open boundaries, which it doesn't

Then why does my test pass?

@jngrad
Copy link
Member Author

jngrad commented Dec 17, 2020

Also, if you test DDS with replica using 0 replica against ScaFaCoS direct summation, you get the same results after energy minimization (using the diff in the second post of this PR), whereas DDS with open borders doesn't match ScaFaCoS anymore.

@RudolfWeeber
Copy link
Contributor

dds_cpu and dds_gpu use minimum image convention in case of perioidic bc, dds with replica and scafacos don't.

@RudolfWeeber RudolfWeeber mentioned this pull request Dec 18, 2020
@kodiakhq kodiakhq bot closed this in #4061 Dec 19, 2020
kodiakhq bot added a commit that referenced this pull request Dec 19, 2020
fixes #1569 closes #4046 

This is based on #4046, minus the last commit.

Description of changes:
* Switch tests for dipolar interactions with open boundary conditions from comparing two methods against each other to comparing individual methods against reference data
* Use prefactor != 1 in dipolar 2d and 3d periodic tests
* Block DipolarDirectSumWithReplicaCpu from n_replica=0 and periodic bc, as it does not apply minimum image convention. Introducing minimum image convention there is not worth it, because that is provided by DipolarDirectSumCpu. The two methods should be merged anyway.
@jngrad jngrad deleted the dds-coverage branch December 23, 2020 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants