Merge pull request #810 from KulaginVladimir/traps-ids-bug

Fix bug with trap id is None

festim/concentration/traps/traps.py

@@ -21,11 +21,6 @@ def __init__(self, *args):
         self.extrinsic_formulations = []
         self.sub_expressions = []
 
-        # add ids if unspecified
-        for i, trap in enumerate(self, 1):
-            if trap.id is None:
-                trap.id = i
-
     @property
     def traps(self):
         warnings.warn(
@@ -71,6 +66,11 @@ def make_traps_materials(self, materials):
         for trap in self:
             trap.make_materials(materials)
 
+    def assign_traps_ids(self):
+        for i, trap in enumerate(self, 1):
+            if trap.id is None:
+                trap.id = i
+
     def create_forms(self, mobile, materials, T, dx, dt=None):
         self.F = 0
         for trap in self:

festim/h_transport_problem.py

@@ -88,6 +88,7 @@ def initialise(self, mesh, materials, dt=None):
         # Define functions
         self.define_function_space(mesh)
         self.initialise_concentrations()
+        self.traps.assign_traps_ids()
         self.traps.make_traps_materials(materials)
         self.traps.initialise_extrinsic_traps(self.V_CG1)
 

test/h_transport_problem/test_initialisation.py

@@ -46,7 +46,7 @@ def test_initialisation_from_xdmf(tmpdir):
             field=1, value=str(Path(file2)), label="2", time_step=0
         ),
     ]
-    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1)
+    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1, 1)
 
     my_problem = festim.HTransportProblem(
         festim.Mobile(),
@@ -84,7 +84,7 @@ def test_initialisation_with_expression():
         festim.InitialCondition(field=0, value=1 + festim.x + festim.y),
         festim.InitialCondition(field=1, value=1 + festim.x),
     ]
-    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1)
+    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1, 1)
 
     my_problem = festim.HTransportProblem(
         festim.Mobile(),
@@ -123,7 +123,7 @@ def test_initialisation_with_expression_chemical_pot():
         festim.InitialCondition(field=0, value=1 + festim.x + festim.y),
         festim.InitialCondition(field=1, value=1 + festim.x),
     ]
-    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1)
+    my_trap = festim.Trap(1, 1, 1, 1, ["mat_name"], 1, 1)
 
     my_theta = festim.Theta()
     my_theta.materials = festim.Materials([festim.Material(1, 1, 0, S_0=S, E_S=0)])

test/simulation/test_postprocessing_integration.py

@@ -18,7 +18,7 @@ def my_sim(self):
         my_sim.materials = festim.Materials([mat1])
 
         my_sim.mobile = festim.Mobile()
-        trap_1 = festim.Trap(1, 1, 1, 1, mat1, 1)
+        trap_1 = festim.Trap(1, 1, 1, 1, mat1, 1, 1)
         my_sim.traps = festim.Traps([trap_1])
 
         my_sim.mesh.define_measures(my_sim.materials)

test/system/test_misc.py

@@ -696,3 +696,36 @@ def test_catch_bug_738(tmpdir):
 
     assert os.path.exists(filename)
     assert os.path.exists(str(tmpdir.join("mobile_re.h5")))
+
+
+def test_catch_bug_804():
+    """
+    Test to catch bug #804
+    Set up a simple simulation with traps. Modify the list of traps, so the some
+    traps in the traps list have id = None. Initialise the simulation.
+
+    We then check that missing trap ids are assigned with values.
+    """
+    model = F.Simulation()
+    model.mesh = F.MeshFromVertices(np.linspace(0, 1, num=100))
+
+    model.materials = F.Material(1, 1, 0)
+
+    model.traps = [F.Trap(1, 1, 1, 1, model.materials[0], 1)]
+
+    model.traps.append(F.Trap(1, 1, 1, 1, model.materials[0], 1))
+    model.traps.extend([F.Trap(1, 1, 1, 1, model.materials[0], 1)])
+    model.traps.insert(2, F.Trap(1, 1, 1, 1, model.materials[0], 1))
+
+    model.T = 500
+
+    model.settings = F.Settings(
+        absolute_tolerance=1e-10,
+        relative_tolerance=1e-10,
+        transient=False,
+    )
+
+    model.initialise()
+
+    for trap in model.traps:
+        assert trap.id is not None

test/system/test_system.py

@@ -209,7 +209,7 @@ def run(h):
         my_materials = festim.Materials(
             [festim.Material(name="mat", id=1, D_0=D_0, E_D=E_D)]
         )
-        my_traps = festim.Traps([festim.Trap(k_0, E_k, p_0, E_p, "mat", n_trap)])
+        my_traps = festim.Traps([festim.Trap(k_0, E_k, p_0, E_p, "mat", n_trap, 1)])
 
         my_initial_conditions = [
             festim.InitialCondition(field=0, value=u),
@@ -335,7 +335,7 @@ def run(h):
         my_materials = festim.Materials(
             [festim.Material(name="mat", id=1, D_0=D_0, E_D=E_D, S_0=2, E_S=0.1)]
         )
-        my_traps = festim.Traps([festim.Trap(k_0, E_k, p_0, E_p, "mat", n_trap)])
+        my_traps = festim.Traps([festim.Trap(k_0, E_k, p_0, E_p, "mat", n_trap, 1)])
 
         my_initial_conditions = [
             festim.InitialCondition(field=0, value=u),
@@ -616,7 +616,7 @@ def run(h):
             [festim.Material(name="mat", id=1, D_0=D_0, E_D=E_D)]
         )
 
-        my_trap = festim.Trap(k_0, E_k, p_0, E_p, ["mat"], n_trap)
+        my_trap = festim.Trap(k_0, E_k, p_0, E_p, ["mat"], n_trap, 1)
 
         my_initial_conditions = [
             festim.InitialCondition(field=0, value=u),
@@ -1158,7 +1158,7 @@ def test_MMS_decay_with_trap():
         [festim.Material(name="mat", id=1, D_0=D_0, E_D=E_D)]
     )
 
-    my_trap = festim.Trap(k_0, E_k, p_0, E_p, ["mat"], n_trap)
+    my_trap = festim.Trap(k_0, E_k, p_0, E_p, ["mat"], n_trap, 1)
 
     size = 0.1
     my_mesh = festim.MeshFromVertices(np.linspace(0, size, 1600))

test/unit/test_traps/test_traps.py

@@ -96,9 +96,13 @@ class TestGetTrap:
     dx = f.dx()
     dt = f.Constant(1)
 
-    trap1 = festim.Trap(k_0=1, E_k=2, p_0=1, E_p=2, materials="mat_name", density=1)
+    trap1 = festim.Trap(
+        k_0=1, E_k=2, p_0=1, E_p=2, materials="mat_name", density=1, id=1
+    )
     add_functions(trap1, V, id=1)
-    trap2 = festim.Trap(k_0=2, E_k=3, p_0=1, E_p=2, materials="mat_name", density=1)
+    trap2 = festim.Trap(
+        k_0=2, E_k=3, p_0=1, E_p=2, materials="mat_name", density=1, id=2
+    )
     add_functions(trap2, V, id=2)
     my_traps = festim.Traps([trap1, trap2])