Skip to content

Commit

Permalink
BUG: update_nnps called too many times from group.
Browse files Browse the repository at this point in the history
This happens when one sets `update_nnps=True` in a Group with multiple
destinations.  Instead of calling update_nnps once at the end, we were
calling it one for each destination which is pointless and wasteful.
This is now tested and fixed on CPU and GPU.
  • Loading branch information
prabhuramachandran committed Mar 14, 2021
1 parent 3bc458d commit bc5c793
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 9 deletions.
4 changes: 1 addition & 3 deletions pysph/sph/acceleration_eval_cython.mako
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ ${indent(all_eqs.get_reduce_code(), 0)}
# Destination ${dest} done.
# ---------------------------------------------------------------------
% endfor
#######################################################################
## Update NNPS locally if needed
#######################################################################
Expand All @@ -129,8 +129,6 @@ with profile_ctx("Integrator.update_domain"):
with profile_ctx("nnps.update"):
nnps.update()
% endif
% endfor
#######################################################################
## Call any `post` functions
#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion pysph/sph/acceleration_eval_gpu.mako
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ ${helper.get_post_loop_kernel(g_idx, sg_idx, group, dest, all_eqs)}
<% helper.call_reduce(all_eqs, dest) %>
% endif
// Finished destination ${dest}.
% endfor
#######################################################################
## Update NNPS locally if needed
#######################################################################
% if group.update_nnps:
<% helper.call_update_nnps(group) %>
% endif
% endfor
#######################################################################
## Call any `post` functions
#######################################################################
Expand Down
110 changes: 105 additions & 5 deletions pysph/sph/tests/test_acceleration_eval.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
# Standard library imports.
try:
# This is for Python-2.6.x
import unittest2 as unittest
except ImportError:
import unittest
import unittest

# Library imports.
import pytest
Expand All @@ -13,6 +9,8 @@
from pysph.base.utils import get_particle_array
from compyle.config import get_config
from compyle.api import declare
from compyle.profile import get_profile_info

from pysph.sph.equation import Equation, Group
from pysph.sph.acceleration_eval import (
AccelerationEval, MegaGroup, CythonGroup,
Expand Down Expand Up @@ -1231,3 +1229,105 @@ def test_update_nnps_is_called_on_gpu(self):
call = h.calls[4]
assert call['type'] == 'kernel'
assert call['loop'] is True


class TestAEvalMultipleDests(unittest.TestCase):
def setUp(self):
self.dim = 1
n = 10
dx = 1.0 / (n - 1)
x = np.linspace(0, 1, n)
m = np.ones_like(x)
h = np.ones_like(x) * dx * 1.05
self.pa1 = get_particle_array(name='f1', x=x, h=h, m=m)
self.pa2 = get_particle_array(name='f2', x=x + dx/2, h=h, m=m)

def _make_accel_eval(self, equations, cache_nnps=False):
arrays = [self.pa1, self.pa2]
kernel = CubicSpline(dim=self.dim)
a_eval = AccelerationEval(
particle_arrays=arrays, equations=equations, kernel=kernel
)
comp = SPHCompiler(a_eval, integrator=None)
comp.compile()
nnps = NNPS(dim=kernel.dim, particles=arrays, cache=cache_nnps)
nnps.update()
a_eval.set_nnps(nnps)
return a_eval

def test_update_nnps_should_only_be_called_once_per_group(self):
# Given
eqs = [
SummationDensity(dest='f1', sources=['f1', 'f2']),
SummationDensity(dest='f2', sources=['f1', 'f2']),
]
equations = [Group(equations=eqs, update_nnps=True)]
a_eval = self._make_accel_eval(equations)

# This is quite ugly, it is not easy to mock out the set_nnps on an
# acceleration eval as that requires a Cython object and not a mock
# instance.
profile_info = get_profile_info()
n_up_dom = profile_info[0]['Integrator.update_domain']['calls']
n_up = profile_info[0]['nnps.update']['calls']
# When
a_eval.compute(0.1, 0.1)

# Then
profile_info = get_profile_info()
ncall = profile_info[0]['Integrator.update_domain']['calls']
self.assertEqual(ncall, n_up_dom + 1)
ncall = profile_info[0]['nnps.update']['calls']
self.assertEqual(ncall, n_up + 1)


class TestAEvalMultipleDestsGPU(unittest.TestCase):
def setUp(self):
self.dim = 1
n = 10
dx = 1.0 / (n - 1)
x = np.linspace(0, 1, n)
m = np.ones_like(x)
h = np.ones_like(x) * dx * 1.05
self.pa1 = get_particle_array(name='f1', x=x, h=h, m=m)
self.pa2 = get_particle_array(name='f2', x=x + dx/2, h=h, m=m)

def _get_nnps_cls(self):
from pysph.base.gpu_nnps import ZOrderGPUNNPS as GPUNNPS
return GPUNNPS

def _make_accel_eval(self, equations, cache_nnps=True):
pytest.importorskip('pyopencl')
pytest.importorskip('pysph.base.gpu_nnps')
GPUNNPS = self._get_nnps_cls()
arrays = [self.pa1, self.pa2]
kernel = CubicSpline(dim=self.dim)
a_eval = AccelerationEval(
particle_arrays=arrays, equations=equations, kernel=kernel,
backend='opencl'
)
comp = SPHCompiler(a_eval, integrator=None)
comp.compile()
self.sph_compiler = comp
nnps = GPUNNPS(dim=kernel.dim, particles=arrays, cache=cache_nnps,
backend='opencl')
nnps.update()
a_eval.set_nnps(nnps)
return a_eval

def test_update_nnps_should_only_be_called_once_per_group(self):
# Given
eqs = [
SummationDensity(dest='f1', sources=['f1', 'f2']),
SummationDensity(dest='f2', sources=['f1', 'f2']),
]
equations = [Group(equations=eqs, update_nnps=True)]

# When
a_eval = self._make_accel_eval(equations)

# Then
h = a_eval.c_acceleration_eval.helper
update_nnps = [call for call in h.calls
if call['method'] == 'update_nnps']
self.assertEqual(len(update_nnps), 1)

0 comments on commit bc5c793

Please sign in to comment.