Repository: OceanParcels/parcels (python)

Issue:
c-code generator for if-statements inside custom kernels
Hi all,

There seems to be a bug in the c-code generator when using if-statements inside a custom kernel. I'll attach a sample script based on the stommel example and the corresponding c-code.

When I put an if-statement inside of another if-statement, the code inside of the if-statement (see l. 134-136 c-code) seems to get executed before the if-statement itself is checked (l. 137). The corresponding if-statement in the python code can be found on l. 91-92.

Best,
Mikael

[0d46b78f1f104285b3dc199479ae9038.c.txt](https://github.com/OceanParcels/parcels/files/2884289/0d46b78f1f104285b3dc199479ae9038.c.txt)

[Stommel.py.txt](https://github.com/OceanParcels/parcels/files/2884293/Stommel.py.txt)


Pull request description:
Bug-fix for erroneous behaviour when 'tmp' used in custom kernels
This PR fixes #538, which was caused by a user variable with the string `tmp` in the name. These are also used in the codegenerator for Field interpolation, creating erroneous behaviour.

Now, this is fixed by
- changing the special`tmp` to `parcels_tmpvar` in the code generator, which users are far less likely to use themselves
- throwing an error if users do use `parcels_tmpvar` in a custom kernel


Patch:
diff --git a/parcels/codegenerator.py b/parcels/codegenerator.py
index 47695ea7..08001043 100644
--- a/parcels/codegenerator.py
+++ b/parcels/codegenerator.py
@@ -209,7 +209,7 @@ class IntrinsicTransformer(ast.NodeTransformer):
 
     def get_tmp(self):
         """Create a new temporary veriable name"""
-        tmp = "tmp%d" % self._tmp_counter
+        tmp = "parcels_tmpvar%d" % self._tmp_counter
         self._tmp_counter += 1
         self.tmp_vars += [tmp]
         return tmp
@@ -228,6 +228,8 @@ class IntrinsicTransformer(ast.NodeTransformer):
             node = RandomNode(math, ccode='')
         elif node.id == 'print':
             node = PrintNode()
+        elif 'parcels_tmpvar' in node.id:
+            raise NotImplementedError("Custom Kernels cannot contain string 'parcels_tmpvar'; please change your kernel")
         return node
 
     def visit_Attribute(self, node):
@@ -542,7 +544,7 @@ class KernelGenerator(ast.NodeVisitor):
             self.visit(b)
         # field evals are replaced by a tmp variable is added to the stack.
         # Here it means field evals passes from node.test to node.body. We take it out manually
-        fieldInTestCount = node.test.ccode.count('tmp')
+        fieldInTestCount = node.test.ccode.count('parcels_tmpvar')
         body0 = c.Block([b.ccode for b in node.body[:fieldInTestCount]])
         body = c.Block([b.ccode for b in node.body[fieldInTestCount:]])
         orelse = c.Block([b.ccode for b in node.orelse]) if len(node.orelse) > 0 else None


Test patch:
diff --git a/tests/test_kernel_language.py b/tests/test_kernel_language.py
index a87f4d56..874bc981 100644
--- a/tests/test_kernel_language.py
+++ b/tests/test_kernel_language.py
@@ -110,6 +110,39 @@ def test_while_if_break(fieldset, mode):
     assert np.allclose(np.array([p.p for p in pset]), 20., rtol=1e-12)
 
 
+@pytest.mark.parametrize('mode', ['scipy', 'jit'])
+def test_nested_if(fieldset, mode):
+    """Test nested if commands"""
+    class TestParticle(ptype[mode]):
+        p0 = Variable('p0', dtype=np.int32, initial=0)
+        p1 = Variable('p1', dtype=np.int32, initial=1)
+    pset = ParticleSet(fieldset, pclass=TestParticle, lon=0, lat=0)
+
+    def kernel(particle, fieldset, time):
+        if particle.p1 >= particle.p0:
+            var = particle.p0
+            if var + 1 < particle.p1:
+                particle.p1 = -1
+
+    pset.execute(kernel, endtime=10, dt=1.)
+    assert np.allclose([pset[0].p0, pset[0].p1], [0, 1])
+
+
+def test_parcels_tmpvar_in_kernel(fieldset):
+    """Tests for error thrown if vartiable with 'tmp' defined in custom kernel"""
+    error_thrown = False
+    pset = ParticleSet(fieldset, pclass=JITParticle, lon=0, lat=0)
+
+    def kernel_tmpvar(particle, fieldset, time):
+        parcels_tmpvar0 = 0  # noqa
+
+    try:
+        pset.execute(kernel_tmpvar, endtime=1, dt=1.)
+    except NotImplementedError:
+        error_thrown = True
+    assert error_thrown
+
+
 @pytest.mark.parametrize('mode', ['scipy', 'jit'])
 def test_if_withfield(fieldset, mode):
     """Test combination of if and Field sampling commands"""