From df7a8343c69935df9257c41649757319bf56c20e Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 20:23:51 +0100 Subject: [PATCH 01/38] Allowing an arbitrary product number to be present in a reaction. --- festim/hydrogen_transport_problem.py | 3 +++ festim/reaction.py | 15 ++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index af0efd000..e3ab3467c 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -670,8 +670,11 @@ def create_formulation(self): * self.dx(reaction.volume.id) ) # product + if isinstance(reaction.product, list): products = reaction.product + elif not reaction.product: + products = [] else: products = [reaction.product] for product in products: diff --git a/festim/reaction.py b/festim/reaction.py index 73abb1102..c5c4619e0 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -115,14 +115,19 @@ def reaction_term(self, temperature): c_A = self.reactant1.concentration c_B = self.reactant2.concentration - + if isinstance(self.product, list): products = self.product + elif not self.product: + products = [] else: products = [self.product] - products_of_product = products[0].solution - for product in products[1:]: - products_of_product *= product.solution - + if len(products) > 0: + products_of_product = products[0].solution + for product in products: + products_of_product *= product.solution + else: + products_of_product = 0 + return k * c_A * c_B - p * products_of_product From f4c49038c6d5341cbc459036ff39f2f85fde638b Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 20:40:40 +0100 Subject: [PATCH 02/38] Updating docstrings and typehints --- festim/reaction.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index c5c4619e0..a5a957ad0 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -1,5 +1,5 @@ import festim as F -from typing import Union +from typing import Union, Optional from ufl import exp @@ -10,7 +10,7 @@ class Reaction: Arguments: reactant1 (Union[F.Species, F.ImplicitSpecies]): The first reactant. reactant2 (Union[F.Species, F.ImplicitSpecies]): The second reactant. - product (F.Species): The product. + product (Optional[F.Species]): The product. k_0 (float): The forward rate constant pre-exponential factor. E_k (float): The forward rate constant activation energy. p_0 (float): The backward rate constant pre-exponential factor. @@ -20,7 +20,7 @@ class Reaction: Attributes: reactant1 (Union[F.Species, F.ImplicitSpecies]): The first reactant. reactant2 (Union[F.Species, F.ImplicitSpecies]): The second reactant. - product (F.Species): The product. + product (Optional[F.Species]): The product. k_0 (float): The forward rate constant pre-exponential factor. E_k (float): The forward rate constant activation energy. p_0 (float): The backward rate constant pre-exponential factor. @@ -50,7 +50,7 @@ def __init__( self, reactant1: Union[F.Species, F.ImplicitSpecies], reactant2: Union[F.Species, F.ImplicitSpecies], - product: F.Species, + product: Optional[F.Species], k_0: float, E_k: float, p_0: float, From 000112129fa71aba5b722a897cc726fef236d1d9 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:03:54 +0100 Subject: [PATCH 03/38] Enabling ability for an arbitrary number of reactants. --- festim/hydrogen_transport_problem.py | 18 +++---- festim/reaction.py | 74 +++++++++++++++------------- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index e3ab3467c..e42e870b9 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -655,20 +655,18 @@ def create_formulation(self): self.formulation += ((u - u_n) / self.dt) * v * self.dx(vol.id) for reaction in self.reactions: - # reactant 1 - if isinstance(reaction.reactant1, F.Species): - self.formulation += ( - reaction.reaction_term(self.temperature_fenics) - * reaction.reactant1.test_function - * self.dx(reaction.volume.id) - ) - # reactant 2 - if isinstance(reaction.reactant2, F.Species): + # reactant + if isinstance(reaction.reactant, list): + reactants = reaction.reactant + else: + reactants = [reaction.reactant] + for reactant in reactants: self.formulation += ( reaction.reaction_term(self.temperature_fenics) - * reaction.reactant2.test_function + * reactant.test_function * self.dx(reaction.volume.id) ) + # product if isinstance(reaction.product, list): diff --git a/festim/reaction.py b/festim/reaction.py index a5a957ad0..efc0f7a59 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -48,8 +48,7 @@ class Reaction: def __init__( self, - reactant1: Union[F.Species, F.ImplicitSpecies], - reactant2: Union[F.Species, F.ImplicitSpecies], + reactant: Union[F.Species, F.ImplicitSpecies], product: Optional[F.Species], k_0: float, E_k: float, @@ -57,8 +56,7 @@ def __init__( E_p: float, volume: F.VolumeSubdomain1D, ) -> None: - self.reactant1 = reactant1 - self.reactant2 = reactant2 + self.reactant = reactant self.product = product self.k_0 = k_0 self.E_k = E_k @@ -67,42 +65,42 @@ def __init__( self.volume = volume @property - def reactant1(self): - return self._reactant1 - - @reactant1.setter - def reactant1(self, value): - if not isinstance(value, (F.Species, F.ImplicitSpecies)): - raise TypeError( - f"reactant1 must be an F.Species or F.ImplicitSpecies, not {type(value)}" - ) - self._reactant1 = value - - @property - def reactant2(self): - return self._reactant2 - - @reactant2.setter - def reactant2(self, value): - if not isinstance(value, (F.Species, F.ImplicitSpecies)): - raise TypeError( - f"reactant2 must be an F.Species or F.ImplicitSpecies, not {type(value)}" - ) - self._reactant2 = value + def reactant(self): + return self._reactant + + @reactant.setter + def reactant(self, value): + if not isinstance(value, list): + value = [value] + for i in value: + if not isinstance(i, (F.Species, F.ImplicitSpecies)): + raise TypeError( + f"reactant must be an F.Species or F.ImplicitSpecies, not {type(i)}" + ) + self._reactant = value def __repr__(self) -> str: + if isinstance(self.reactant, list): + reactants = " + ".join([str(reactant) for reactant in self.reactant]) + else: + reactants = self.reactant + if isinstance(self.product, list): products = " + ".join([str(product) for product in self.product]) else: products = self.product - return f"Reaction({self.reactant1} + {self.reactant2} <--> {products}, {self.k_0}, {self.E_k}, {self.p_0}, {self.E_p})" + return f"Reaction({reactants} <--> {products}, {self.k_0}, {self.E_k}, {self.p_0}, {self.E_p})" def __str__(self) -> str: + if isinstance(self.reactant, list): + reactants = " + ".join([str(reactant) for reactant in self.reactant]) + else: + reactants = self.reactant if isinstance(self.product, list): products = " + ".join([str(product) for product in self.product]) else: products = self.product - return f"{self.reactant1} + {self.reactant2} <--> {products}" + return f"{reactants} <--> {products}" def reaction_term(self, temperature): """Compute the reaction term at a given temperature. @@ -113,8 +111,16 @@ def reaction_term(self, temperature): k = self.k_0 * exp(-self.E_k / (F.k_B * temperature)) p = self.p_0 * exp(-self.E_p / (F.k_B * temperature)) - c_A = self.reactant1.concentration - c_B = self.reactant2.concentration + if isinstance(self.reactant, list): + reactants = self.reactant + elif not self.reactant: + reactants = [] + else: + reactants = [self.reactant] + + product_of_reactants = reactants[0].solution + for reactant in reactants: + product_of_reactants *= reactant.solution if isinstance(self.product, list): products = self.product @@ -124,10 +130,10 @@ def reaction_term(self, temperature): products = [self.product] if len(products) > 0: - products_of_product = products[0].solution + product_of_products = products[0].solution for product in products: - products_of_product *= product.solution + product_of_products *= product.solution else: - products_of_product = 0 + product_of_products = 0 - return k * c_A * c_B - p * products_of_product + return k * product_of_reactants - p * product_of_products From 6f5457dd29b82120e9a33dba3bc89ed358c32a38 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:06:31 +0100 Subject: [PATCH 04/38] Raise ValueError if len(reactant)=0 --- festim/reaction.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/festim/reaction.py b/festim/reaction.py index efc0f7a59..a900af04d 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -72,6 +72,8 @@ def reactant(self): def reactant(self, value): if not isinstance(value, list): value = [value] + if len(value) == 0: + raise ValueError(f"reactant must be an entry of one or more species objects, not zero.") for i in value: if not isinstance(i, (F.Species, F.ImplicitSpecies)): raise TypeError( From ad3b1b475063b2a53b8050d61c4d4af355c55ab4 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:12:49 +0100 Subject: [PATCH 05/38] Updating doc-strings and type hints --- festim/reaction.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index a900af04d..09987584d 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -1,5 +1,5 @@ import festim as F -from typing import Union, Optional +from typing import Union, Optional, List from ufl import exp @@ -8,9 +8,8 @@ class Reaction: """A reaction between two species, with a forward and backward rate. Arguments: - reactant1 (Union[F.Species, F.ImplicitSpecies]): The first reactant. - reactant2 (Union[F.Species, F.ImplicitSpecies]): The second reactant. - product (Optional[F.Species]): The product. + reactant (Union[F.Species, F.ImplicitSpecies], List[Union[F.Species, F.ImplicitSpecies]]): The reactant. + product (Optional[Union[F.Species, List[F.Species]]]): The product. k_0 (float): The forward rate constant pre-exponential factor. E_k (float): The forward rate constant activation energy. p_0 (float): The backward rate constant pre-exponential factor. @@ -18,9 +17,8 @@ class Reaction: volume (F.VolumeSubdomain1D): The volume subdomain where the reaction takes place. Attributes: - reactant1 (Union[F.Species, F.ImplicitSpecies]): The first reactant. - reactant2 (Union[F.Species, F.ImplicitSpecies]): The second reactant. - product (Optional[F.Species]): The product. + reactant (Union[F.Species, F.ImplicitSpecies], List[Union[F.Species, F.ImplicitSpecies]]): The reactant. + product (Optional[Union[F.Species, List[F.Species]]]): The product. k_0 (float): The forward rate constant pre-exponential factor. E_k (float): The forward rate constant activation energy. p_0 (float): The backward rate constant pre-exponential factor. @@ -29,14 +27,13 @@ class Reaction: Usage: >>> # create two species - >>> species1 = F.Species("A") - >>> species2 = F.Species("B") + >>> reactant = [F.Species("A"), F.Species("B")] >>> # create a product species >>> product = F.Species("C") >>> # create a reaction between the two species - >>> reaction = Reaction(species1, species2, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3) + >>> reaction = Reaction(reactant, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3) >>> print(reaction) A + B <--> C From 9514d6166bfabda1df3536e0285705f617ab18f2 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:27:02 +0100 Subject: [PATCH 06/38] Updating current tests --- test/system_tests/test_1_mobile_1_trap.py | 9 ++-- .../test_reaction_not_in_every_volume.py | 3 +- test/test_reaction.py | 49 +++++-------------- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/test/system_tests/test_1_mobile_1_trap.py b/test/system_tests/test_1_mobile_1_trap.py index 58aeb073c..b08f51f7b 100644 --- a/test/system_tests/test_1_mobile_1_trap.py +++ b/test/system_tests/test_1_mobile_1_trap.py @@ -76,8 +76,7 @@ def v_exact(mod): my_model.reactions = [ F.Reaction( - reactant1=mobile, - reactant2=traps, + reactant=[mobile, traps], product=trapped, k_0=k_0, E_k=E_k, @@ -273,8 +272,7 @@ def v_exact(mod): my_model.reactions = [ F.Reaction( - reactant1=mobile, - reactant2=traps, + reactant=[mobile, traps], product=trapped, k_0=k_0, E_k=E_k, @@ -403,8 +401,7 @@ def v_exact(mod): my_model.reactions = [ F.Reaction( - reactant1=mobile, - reactant2=traps, + reactant=[mobile, traps], product=trapped, k_0=k_0, E_k=E_k, diff --git a/test/system_tests/test_reaction_not_in_every_volume.py b/test/system_tests/test_reaction_not_in_every_volume.py index 5b7a5c3e6..ea315458a 100644 --- a/test/system_tests/test_reaction_not_in_every_volume.py +++ b/test/system_tests/test_reaction_not_in_every_volume.py @@ -19,8 +19,7 @@ def test_sim_reaction_not_in_every_volume(): trap = F.ImplicitSpecies(n=8.19e25, others=[ct]) my_model.reactions = [ F.Reaction( - reactant1=cm, - reactant2=trap, + reactant=[cm, trap], product=ct, k_0=8.9e-17, E_k=0.39, diff --git a/test/test_reaction.py b/test/test_reaction.py index 741790efc..6353b9720 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -19,12 +19,11 @@ def test_reaction_init(): # create a reaction between the two species reaction = F.Reaction( - species1, species2, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + [species1, species], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol ) # check that the attributes are set correctly - assert reaction.reactant1 == species1 - assert reaction.reactant2 == species2 + assert reaction.reactant == [species1, species2] assert reaction.product == product assert reaction.k_0 == 1.0 assert reaction.E_k == 0.2 @@ -44,7 +43,7 @@ def test_reaction_repr(): # create a reaction between the two species reaction = F.Reaction( - species1, species2, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol ) # check that the __repr__ method returns the expected string @@ -65,8 +64,7 @@ def test_reaction_repr_2_products(): # create a reaction between the two species reaction = F.Reaction( - species1, - species2, + [species1, species2], [product1, product2], k_0=1.0, E_k=0.2, @@ -92,7 +90,7 @@ def test_reaction_str(): # create a reaction between the two species reaction = F.Reaction( - species1, species2, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol ) # check that the __str__ method returns the expected string @@ -113,8 +111,7 @@ def test_reaction_str_2_products(): # create a reaction between the two species reaction = F.Reaction( - species1, - species2, + [species1, species2], [product1, product2], k_0=1.0, E_k=0.2, @@ -147,7 +144,7 @@ def test_reaction_reaction_term(temperature): # create a reaction between the two species reaction = F.Reaction( - species1, species2, product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol ) # test the reaction term at a given temperature @@ -185,8 +182,7 @@ def test_reaction_reaction_term_2_products(temperature): # create a reaction between the two species reaction = F.Reaction( - species1, - species2, + [species1, species2], [product1, product2], k_0=1.0, E_k=0.2, @@ -209,37 +205,18 @@ def arrhenius(pre, act, T): assert reaction.reaction_term(temperature) == expected_reaction_term -def test_reactant1_setter_raises_error_with_wrong_type(): - """Test a type error is raised when the reactant1 is given a wrong type.""" +def test_reactant_setter_raises_error_with_wrong_type(): + """Test a type error is raised when the first reactant is given a wrong type.""" with pytest.raises( TypeError, - match="reactant1 must be an F.Species or F.ImplicitSpecies, not ", + match="reactant must be an F.Species or F.ImplicitSpecies, not ", ): F.Reaction( - reactant1="A", - reactant2=F.Species("B"), + reactant=["A", F.Species("B")], product=F.Species("C"), k_0=1, E_k=0.1, p_0=2, E_p=0.2, volume=my_vol, - ) - - -def test_reactant2_setter_raises_error_with_wrong_type(): - """Test a type error is raised when the reactant2 is given a wrong type.""" - with pytest.raises( - TypeError, - match="reactant2 must be an F.Species or F.ImplicitSpecies, not ", - ): - F.Reaction( - reactant1=F.Species("A"), - reactant2="B", - product=F.Species("C"), - k_0=1, - E_k=0.1, - p_0=2, - E_p=0.2, - volume=my_vol, - ) + ) \ No newline at end of file From 6d40e359181243c9877e6c11d47436e43256e2d5 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:28:21 +0100 Subject: [PATCH 07/38] Updating the example files. --- examples/multi_isotope_trapping_example.py | 18 ++++++------------ examples/tds_example.py | 6 ++---- examples/trapping_example.py | 3 +-- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/examples/multi_isotope_trapping_example.py b/examples/multi_isotope_trapping_example.py index a3528629e..63a64eea8 100644 --- a/examples/multi_isotope_trapping_example.py +++ b/examples/multi_isotope_trapping_example.py @@ -62,8 +62,7 @@ E_k=0.39, p_0=1e13, E_p=1.2, - reactant1=mobile_H, - reactant2=empty_trap, + reactant=[mobile_H, empty_trap], product=trapped_H1, ), F.Reaction( @@ -71,8 +70,7 @@ E_k=0.39, p_0=1e13, E_p=1.0, - reactant1=mobile_H, - reactant2=trapped_H1, + reactant=[mobile_H, trapped_H1], product=trapped_H2, ), F.Reaction( @@ -80,8 +78,7 @@ E_k=0.39, p_0=1e13, E_p=1.2, - reactant1=mobile_D, - reactant2=empty_trap, + reactant=[mobile_D, empty_trap], product=trapped_D1, ), F.Reaction( @@ -89,8 +86,7 @@ E_k=0.39, p_0=1e13, E_p=1.2, - reactant1=mobile_D, - reactant2=trapped_D1, + reactant1=[mobile_D, trapped_D1], product=trapped_D2, ), F.Reaction( @@ -98,8 +94,7 @@ E_k=0.39, p_0=1e13, E_p=1.0, - reactant1=mobile_H, - reactant2=trapped_D1, + reactant=[mobile_H, trapped_D1], product=trapped_HD, ), F.Reaction( @@ -107,8 +102,7 @@ E_k=0.39, p_0=1e13, E_p=1.0, - reactant1=mobile_D, - reactant2=trapped_H1, + reactant1=[mobile_D, trapped_H1], product=trapped_HD, ), ] diff --git a/examples/tds_example.py b/examples/tds_example.py index 0d76eed08..d0cf87013 100644 --- a/examples/tds_example.py +++ b/examples/tds_example.py @@ -52,8 +52,7 @@ E_k=0.39, p_0=1e13, E_p=0.87, - reactant1=mobile_H, - reactant2=empty_trap1, + reactant=[mobile_H, empty_trap1], product=trapped_H1, volume=my_subdomain, ), @@ -62,8 +61,7 @@ E_k=0.39, p_0=1e13, E_p=1.0, - reactant1=mobile_H, - reactant2=empty_trap2, + reactant=[mobile_H, empty_trap2], product=trapped_H2, volume=my_subdomain, ), diff --git a/examples/trapping_example.py b/examples/trapping_example.py index 8d36fc6d3..2ed82a7d7 100644 --- a/examples/trapping_example.py +++ b/examples/trapping_example.py @@ -30,8 +30,7 @@ E_p=1.2, k_0=3.8e-17, E_k=0.2, - reactant1=mobile_H, - reactant2=empty_trap, + reactant=[mobile_H, empty_trap], product=trapped_H, ) ] From 0ba493d74e865665b20a88153c40c7b9fc803824 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:36:13 +0100 Subject: [PATCH 08/38] Added tests for A -> None reactions --- test/test_reaction.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/test_reaction.py b/test/test_reaction.py index 6353b9720..acd236529 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -78,6 +78,27 @@ def test_reaction_repr_2_products(): assert repr(reaction) == expected_repr +def test_reaction_repr_2_products(): + """Test that the Reaction __repr__ method returns the expected string""" + + # create two species + species1 = F.Species("A") + + # create a reaction between the two species + reaction = F.Reaction( + species1, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, + ) + + # check that the __repr__ method returns the expected string + expected_repr = "Reaction(A <--> None, 1.0, 0.2, 0.1, 0.3)" + assert repr(reaction) == expected_repr + + def test_reaction_str(): """Test that the Reaction __str__ method returns the expected string""" @@ -124,6 +145,26 @@ def test_reaction_str_2_products(): expected_str = "A + B <--> C + D" assert str(reaction) == expected_str +def test_reaction_str_no_products(): + """Test that the Reaction __str__ method returns the expected string when there are 2 products""" + + # create two species + species1 = F.Species("A") + + # create a reaction between the two species + reaction = F.Reaction( + species1, + None, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, + ) + + # check that the __str__ method returns the expected string + expected_str = "A <--> None" + assert str(reaction) == expected_str @pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) def test_reaction_reaction_term(temperature): From d75f3f542987b9b9ff24136337d04997a76820e6 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 21:39:28 +0100 Subject: [PATCH 09/38] Black formatting --- festim/hydrogen_transport_problem.py | 6 +++--- festim/reaction.py | 22 ++++++++++++---------- test/test_reaction.py | 4 +++- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index e42e870b9..1577192be 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -666,12 +666,12 @@ def create_formulation(self): * reactant.test_function * self.dx(reaction.volume.id) ) - + # product - + if isinstance(reaction.product, list): products = reaction.product - elif not reaction.product: + elif not reaction.product: products = [] else: products = [reaction.product] diff --git a/festim/reaction.py b/festim/reaction.py index 09987584d..cff848900 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -67,10 +67,12 @@ def reactant(self): @reactant.setter def reactant(self, value): - if not isinstance(value, list): + if not isinstance(value, list): value = [value] - if len(value) == 0: - raise ValueError(f"reactant must be an entry of one or more species objects, not zero.") + if len(value) == 0: + raise ValueError( + f"reactant must be an entry of one or more species objects, not zero." + ) for i in value: if not isinstance(i, (F.Species, F.ImplicitSpecies)): raise TypeError( @@ -83,7 +85,7 @@ def __repr__(self) -> str: reactants = " + ".join([str(reactant) for reactant in self.reactant]) else: reactants = self.reactant - + if isinstance(self.product, list): products = " + ".join([str(product) for product in self.product]) else: @@ -112,7 +114,7 @@ def reaction_term(self, temperature): if isinstance(self.reactant, list): reactants = self.reactant - elif not self.reactant: + elif not self.reactant: reactants = [] else: reactants = [self.reactant] @@ -120,10 +122,10 @@ def reaction_term(self, temperature): product_of_reactants = reactants[0].solution for reactant in reactants: product_of_reactants *= reactant.solution - + if isinstance(self.product, list): products = self.product - elif not self.product: + elif not self.product: products = [] else: products = [self.product] @@ -132,7 +134,7 @@ def reaction_term(self, temperature): product_of_products = products[0].solution for product in products: product_of_products *= product.solution - else: - product_of_products = 0 - + else: + product_of_products = 0 + return k * product_of_reactants - p * product_of_products diff --git a/test/test_reaction.py b/test/test_reaction.py index acd236529..e6b1516b3 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -145,6 +145,7 @@ def test_reaction_str_2_products(): expected_str = "A + B <--> C + D" assert str(reaction) == expected_str + def test_reaction_str_no_products(): """Test that the Reaction __str__ method returns the expected string when there are 2 products""" @@ -166,6 +167,7 @@ def test_reaction_str_no_products(): expected_str = "A <--> None" assert str(reaction) == expected_str + @pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) def test_reaction_reaction_term(temperature): """Test that the Reaction.reaction_term method returns the expected reaction term""" @@ -260,4 +262,4 @@ def test_reactant_setter_raises_error_with_wrong_type(): p_0=2, E_p=0.2, volume=my_vol, - ) \ No newline at end of file + ) From 3c1381d7d62da820df9c7d339bab0cc2bfa64817 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 22:09:37 +0100 Subject: [PATCH 10/38] Fixing error in tests --- test/test_complex_reaction.py | 3 +-- test/test_reaction.py | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/test/test_complex_reaction.py b/test/test_complex_reaction.py index 6013955f9..dfaaab6a7 100644 --- a/test/test_complex_reaction.py +++ b/test/test_complex_reaction.py @@ -93,8 +93,7 @@ def model_test_reaction(stepsize=1, k=350e-4, p=120e-4, c_A_0=1): E_k=0, p_0=p, E_p=0, - reactant1=species_A, - reactant2=species_B, + reactant=[species_A, species_B], product=[species_C, species_D], volume=my_subdomain, ), diff --git a/test/test_reaction.py b/test/test_reaction.py index e6b1516b3..ea7083f0f 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -19,7 +19,7 @@ def test_reaction_init(): # create a reaction between the two species reaction = F.Reaction( - [species1, species], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol ) # check that the attributes are set correctly @@ -145,7 +145,6 @@ def test_reaction_str_2_products(): expected_str = "A + B <--> C + D" assert str(reaction) == expected_str - def test_reaction_str_no_products(): """Test that the Reaction __str__ method returns the expected string when there are 2 products""" @@ -167,7 +166,6 @@ def test_reaction_str_no_products(): expected_str = "A <--> None" assert str(reaction) == expected_str - @pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) def test_reaction_reaction_term(temperature): """Test that the Reaction.reaction_term method returns the expected reaction term""" @@ -262,4 +260,4 @@ def test_reactant_setter_raises_error_with_wrong_type(): p_0=2, E_p=0.2, volume=my_vol, - ) + ) \ No newline at end of file From dec7172965187c31f72d0cfccb60929d0f093aa3 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 22:36:11 +0100 Subject: [PATCH 11/38] Fixing product iterator --- festim/reaction.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index cff848900..bb451f25c 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -119,9 +119,9 @@ def reaction_term(self, temperature): else: reactants = [self.reactant] - product_of_reactants = reactants[0].solution - for reactant in reactants: - product_of_reactants *= reactant.solution + product_of_reactants = reactants[0].concentration + for reactant in reactants[1:]: + product_of_reactants *= reactant.concentration if isinstance(self.product, list): products = self.product @@ -132,7 +132,7 @@ def reaction_term(self, temperature): if len(products) > 0: product_of_products = products[0].solution - for product in products: + for product in products[1:]: product_of_products *= product.solution else: product_of_products = 0 From a4bc1c56187b3f97723ed4b3148a842f6a14dd55 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 22:50:02 +0100 Subject: [PATCH 12/38] Fixing failed tets --- test/test_reaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index ea7083f0f..5f5060e34 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -196,7 +196,7 @@ def arrhenius(pre, act, T): p = arrhenius(reaction.p_0, reaction.E_p, temperature) expected_reaction_term = ( - k * species1.solution * species2.solution - p * product.solution + k * (species1.solution * species2.solution) - p * product.solution ) assert reaction.reaction_term(temperature) == expected_reaction_term @@ -241,7 +241,7 @@ def arrhenius(pre, act, T): product_of_products = product1.solution * product2.solution expected_reaction_term = ( - k * species1.solution * species2.solution - p * product_of_products + k * (species1.solution * species2.solution) - p * product_of_products ) assert reaction.reaction_term(temperature) == expected_reaction_term From 7bf193e32a4eb53843056d1b4c683a638d14f77d Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 22:50:42 +0100 Subject: [PATCH 13/38] Updating call to reaction in species --- festim/species.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/festim/species.py b/festim/species.py index bc06467b5..f4354b9e1 100644 --- a/festim/species.py +++ b/festim/species.py @@ -117,8 +117,7 @@ class Trap(Species): ct = F.Species("trapped") trap_sites = F.ImplicitSpecies(n=1, others=[ct]) trap_reaction = F.Reaction( - reactant1=cm, - reactant2=trap_sites, + reactant=[cm,trap_sites], product=ct, k_0=1, E_k=1, @@ -153,8 +152,7 @@ def create_species_and_reaction(self): trap_site = F.ImplicitSpecies(n=self.n, others=[self.trapped_concentration]) self.reaction = F.Reaction( - reactant1=self.mobile_species, - reactant2=trap_site, + reactant=[self.mobile_species, trap_site], product=self.trapped_concentration, k_0=self.k_0, E_k=self.E_k, From 47da4370f626610d7580658109997b1a28eea493 Mon Sep 17 00:00:00 2001 From: Allentro Date: Mon, 1 Apr 2024 22:59:07 +0100 Subject: [PATCH 14/38] Black formatting --- test/test_reaction.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index 5f5060e34..277303d4f 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -145,6 +145,7 @@ def test_reaction_str_2_products(): expected_str = "A + B <--> C + D" assert str(reaction) == expected_str + def test_reaction_str_no_products(): """Test that the Reaction __str__ method returns the expected string when there are 2 products""" @@ -166,6 +167,7 @@ def test_reaction_str_no_products(): expected_str = "A <--> None" assert str(reaction) == expected_str + @pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) def test_reaction_reaction_term(temperature): """Test that the Reaction.reaction_term method returns the expected reaction term""" @@ -260,4 +262,4 @@ def test_reactant_setter_raises_error_with_wrong_type(): p_0=2, E_p=0.2, volume=my_vol, - ) \ No newline at end of file + ) From 82a4784361982399b6fb3e7be194ad3e6b7fda64 Mon Sep 17 00:00:00 2001 From: Allentro Date: Tue, 2 Apr 2024 00:22:29 +0100 Subject: [PATCH 15/38] Prevent formulation of ImplicitSpecies --- festim/hydrogen_transport_problem.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 1577192be..d5c028325 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -461,9 +461,11 @@ def assign_functions_to_species(self): ).collapse() for idx, spe in enumerate(self.species): + print(idx, spe) spe.solution = sub_solutions[idx] spe.prev_solution = sub_prev_solution[idx] spe.test_function = sub_test_functions[idx] + print("tf", spe.test_function) def define_meshtags_and_measures(self): """Defines the facet and volume meshtags of the model which are used @@ -661,11 +663,12 @@ def create_formulation(self): else: reactants = [reaction.reactant] for reactant in reactants: - self.formulation += ( - reaction.reaction_term(self.temperature_fenics) - * reactant.test_function - * self.dx(reaction.volume.id) - ) + if isinstance(reactant, F.Species): + self.formulation += ( + reaction.reaction_term(self.temperature_fenics) + * reactant.test_function + * self.dx(reaction.volume.id) + ) # product From ecd905aa7a7806ab377a84a5a1f0f07b35b7a78c Mon Sep 17 00:00:00 2001 From: Allentro Date: Tue, 2 Apr 2024 00:30:37 +0100 Subject: [PATCH 16/38] Updating type hints of Reaction class --- festim/reaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/festim/reaction.py b/festim/reaction.py index bb451f25c..38c8ba864 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -46,7 +46,7 @@ class Reaction: def __init__( self, reactant: Union[F.Species, F.ImplicitSpecies], - product: Optional[F.Species], + product: Optional[Union[F.Species, List[F.Species]]], k_0: float, E_k: float, p_0: float, From bdf4f12c2d5ab2c8f2721c17e6daead9955bd44b Mon Sep 17 00:00:00 2001 From: Allentro Date: Tue, 2 Apr 2024 00:39:00 +0100 Subject: [PATCH 17/38] Updating typehint or reactant input of Reaction class --- festim/reaction.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/festim/reaction.py b/festim/reaction.py index 38c8ba864..b7e46b248 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -45,7 +45,9 @@ class Reaction: def __init__( self, - reactant: Union[F.Species, F.ImplicitSpecies], + reactant: Union[ + F.Species, F.ImplicitSpecies, List[Union[F.Species, F.ImplicitSpecies]] + ], product: Optional[Union[F.Species, List[F.Species]]], k_0: float, E_k: float, From fe65d63457f46a5e99cd49150aa25695beac0d61 Mon Sep 17 00:00:00 2001 From: Allentro Date: Tue, 2 Apr 2024 21:58:05 +0100 Subject: [PATCH 18/38] Fixing product in test function. --- test/test_reaction.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index 277303d4f..3e18718c1 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -78,7 +78,7 @@ def test_reaction_repr_2_products(): assert repr(reaction) == expected_repr -def test_reaction_repr_2_products(): +def test_reaction_repr_0_products(): """Test that the Reaction __repr__ method returns the expected string""" # create two species @@ -87,6 +87,7 @@ def test_reaction_repr_2_products(): # create a reaction between the two species reaction = F.Reaction( species1, + None, k_0=1.0, E_k=0.2, p_0=0.1, @@ -146,7 +147,7 @@ def test_reaction_str_2_products(): assert str(reaction) == expected_str -def test_reaction_str_no_products(): +def test_reaction_str_0_products(): """Test that the Reaction __str__ method returns the expected string when there are 2 products""" # create two species From c5bf901fe7787df47b9e6e4999fae068112c8aed Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:57:10 +0100 Subject: [PATCH 19/38] Update examples/multi_isotope_trapping_example.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- examples/multi_isotope_trapping_example.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/multi_isotope_trapping_example.py b/examples/multi_isotope_trapping_example.py index 63a64eea8..bbf17ef7d 100644 --- a/examples/multi_isotope_trapping_example.py +++ b/examples/multi_isotope_trapping_example.py @@ -86,7 +86,7 @@ E_k=0.39, p_0=1e13, E_p=1.2, - reactant1=[mobile_D, trapped_D1], + reactant=[mobile_D, trapped_D1], product=trapped_D2, ), F.Reaction( From 1567668898f7ff19ed69cccfafbe508a44561f4e Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:57:27 +0100 Subject: [PATCH 20/38] Update examples/multi_isotope_trapping_example.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- examples/multi_isotope_trapping_example.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/multi_isotope_trapping_example.py b/examples/multi_isotope_trapping_example.py index bbf17ef7d..9312e62c9 100644 --- a/examples/multi_isotope_trapping_example.py +++ b/examples/multi_isotope_trapping_example.py @@ -102,7 +102,7 @@ E_k=0.39, p_0=1e13, E_p=1.0, - reactant1=[mobile_D, trapped_H1], + reactant=[mobile_D, trapped_H1], product=trapped_HD, ), ] From 875a6d1891f04e1bbbb254f43771cc5957c0e122 Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:57:53 +0100 Subject: [PATCH 21/38] Update festim/hydrogen_transport_problem.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- festim/hydrogen_transport_problem.py | 1 - 1 file changed, 1 deletion(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index d5c028325..3a3ddcf3e 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -461,7 +461,6 @@ def assign_functions_to_species(self): ).collapse() for idx, spe in enumerate(self.species): - print(idx, spe) spe.solution = sub_solutions[idx] spe.prev_solution = sub_prev_solution[idx] spe.test_function = sub_test_functions[idx] From a6afaec9513816f606e29db9b8105369ddea332d Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 20:58:00 +0100 Subject: [PATCH 22/38] Update festim/hydrogen_transport_problem.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- festim/hydrogen_transport_problem.py | 1 - 1 file changed, 1 deletion(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 3a3ddcf3e..71604c88f 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -464,7 +464,6 @@ def assign_functions_to_species(self): spe.solution = sub_solutions[idx] spe.prev_solution = sub_prev_solution[idx] spe.test_function = sub_test_functions[idx] - print("tf", spe.test_function) def define_meshtags_and_measures(self): """Defines the facet and volume meshtags of the model which are used From 6a5397fb75e3545be43f83b40f842bae849bbc67 Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 21:00:10 +0100 Subject: [PATCH 23/38] Update festim/species.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- festim/species.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/festim/species.py b/festim/species.py index f4354b9e1..04ac3f25f 100644 --- a/festim/species.py +++ b/festim/species.py @@ -117,7 +117,7 @@ class Trap(Species): ct = F.Species("trapped") trap_sites = F.ImplicitSpecies(n=1, others=[ct]) trap_reaction = F.Reaction( - reactant=[cm,trap_sites], + reactant=[cm, trap_sites], product=ct, k_0=1, E_k=1, From 42a899a50700e80801a46bdfeac7248da672db5b Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 21:14:24 +0100 Subject: [PATCH 24/38] Default self.product = product or [] --- festim/hydrogen_transport_problem.py | 3 --- festim/reaction.py | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 71604c88f..925fa3707 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -669,11 +669,8 @@ def create_formulation(self): ) # product - if isinstance(reaction.product, list): products = reaction.product - elif not reaction.product: - products = [] else: products = [reaction.product] for product in products: diff --git a/festim/reaction.py b/festim/reaction.py index b7e46b248..672cde6c9 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -56,7 +56,7 @@ def __init__( volume: F.VolumeSubdomain1D, ) -> None: self.reactant = reactant - self.product = product + self.product = product or [] self.k_0 = k_0 self.E_k = E_k self.p_0 = p_0 From e540546f6e8719a48ebd9afafdb712ef5cdee010 Mon Sep 17 00:00:00 2001 From: Ross Allen <33880803+Allentro@users.noreply.github.com> Date: Wed, 3 Apr 2024 21:14:59 +0100 Subject: [PATCH 25/38] Update festim/reaction.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémi Delaporte-Mathurin <40028739+RemDelaporteMathurin@users.noreply.github.com> --- festim/reaction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/festim/reaction.py b/festim/reaction.py index b7e46b248..672cde6c9 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -56,7 +56,7 @@ def __init__( volume: F.VolumeSubdomain1D, ) -> None: self.reactant = reactant - self.product = product + self.product = product or [] self.k_0 = k_0 self.E_k = E_k self.p_0 = p_0 From e95861971c30122175389976f1e00dc02c3cf147 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 21:18:12 +0100 Subject: [PATCH 26/38] Removing superfluous list conversion --- festim/reaction.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index 672cde6c9..450ae2d0b 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -114,13 +114,6 @@ def reaction_term(self, temperature): k = self.k_0 * exp(-self.E_k / (F.k_B * temperature)) p = self.p_0 * exp(-self.E_p / (F.k_B * temperature)) - if isinstance(self.reactant, list): - reactants = self.reactant - elif not self.reactant: - reactants = [] - else: - reactants = [self.reactant] - product_of_reactants = reactants[0].concentration for reactant in reactants[1:]: product_of_reactants *= reactant.concentration From c2f8d5c6c799cdf29e1a78631a7bbc56f136f559 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 21:21:18 +0100 Subject: [PATCH 27/38] Removing superfluous conversion to list --- festim/reaction.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index 450ae2d0b..f0b601d42 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -120,8 +120,6 @@ def reaction_term(self, temperature): if isinstance(self.product, list): products = self.product - elif not self.product: - products = [] else: products = [self.product] From f9491bde929a9df745a4427b6e19f9f17920490e Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 21:25:50 +0100 Subject: [PATCH 28/38] Removing check for reactant being a list --- festim/reaction.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index f0b601d42..c8a90fc5f 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -83,10 +83,7 @@ def reactant(self, value): self._reactant = value def __repr__(self) -> str: - if isinstance(self.reactant, list): - reactants = " + ".join([str(reactant) for reactant in self.reactant]) - else: - reactants = self.reactant + reactants = " + ".join([str(reactant) for reactant in self.reactant]) if isinstance(self.product, list): products = " + ".join([str(product) for product in self.product]) From f5e2fae15089e0f1773a812e42dc98fecf644ab6 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 22:03:29 +0100 Subject: [PATCH 29/38] Removing None when no products are present --- test/test_reaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index 3e18718c1..cdec716be 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -96,7 +96,7 @@ def test_reaction_repr_0_products(): ) # check that the __repr__ method returns the expected string - expected_repr = "Reaction(A <--> None, 1.0, 0.2, 0.1, 0.3)" + expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.1, 0.3)" assert repr(reaction) == expected_repr @@ -165,7 +165,7 @@ def test_reaction_str_0_products(): ) # check that the __str__ method returns the expected string - expected_str = "A <--> None" + expected_str = "A <--> " assert str(reaction) == expected_str From de7ac3481104cc583a351f0624776788ec1c54aa Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 22:21:35 +0100 Subject: [PATCH 30/38] Adding a test against reactant=[] --- festim/reaction.py | 9 +++------ test/test_reaction.py | 21 +++++++++++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index c8a90fc5f..9ff91562a 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -73,7 +73,7 @@ def reactant(self, value): value = [value] if len(value) == 0: raise ValueError( - f"reactant must be an entry of one or more species objects, not zero." + f"reactant must be an entry of one or more species objects, not an empty list." ) for i in value: if not isinstance(i, (F.Species, F.ImplicitSpecies)): @@ -92,10 +92,7 @@ def __repr__(self) -> str: return f"Reaction({reactants} <--> {products}, {self.k_0}, {self.E_k}, {self.p_0}, {self.E_p})" def __str__(self) -> str: - if isinstance(self.reactant, list): - reactants = " + ".join([str(reactant) for reactant in self.reactant]) - else: - reactants = self.reactant + reactants = " + ".join([str(reactant) for reactant in self.reactant]) if isinstance(self.product, list): products = " + ".join([str(product) for product in self.product]) else: @@ -111,6 +108,7 @@ def reaction_term(self, temperature): k = self.k_0 * exp(-self.E_k / (F.k_B * temperature)) p = self.p_0 * exp(-self.E_p / (F.k_B * temperature)) + reactants = self.reactant product_of_reactants = reactants[0].concentration for reactant in reactants[1:]: product_of_reactants *= reactant.concentration @@ -126,5 +124,4 @@ def reaction_term(self, temperature): product_of_products *= product.solution else: product_of_products = 0 - return k * product_of_reactants - p * product_of_products diff --git a/test/test_reaction.py b/test/test_reaction.py index cdec716be..eea9a6d77 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -96,7 +96,7 @@ def test_reaction_repr_0_products(): ) # check that the __repr__ method returns the expected string - expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.1, 0.3)" + expected_repr = "Reaction(A <--> None, 1.0, 0.2, 0.1, 0.3)" assert repr(reaction) == expected_repr @@ -165,7 +165,7 @@ def test_reaction_str_0_products(): ) # check that the __str__ method returns the expected string - expected_str = "A <--> " + expected_str = "A <--> None" assert str(reaction) == expected_str @@ -249,6 +249,23 @@ def arrhenius(pre, act, T): assert reaction.reaction_term(temperature) == expected_reaction_term +def test_reactant_setter_raises_error_with_zero_length_list(): + """Test a value error is raised when the first reactant is given a wrong type.""" + with pytest.raises( + ValueError, + match="reactant must be an entry of one or more species objects, not an empty list.", + ): + F.Reaction( + reactant=[], + product=None, + k_0=1, + E_k=0.1, + p_0=2, + E_p=0.2, + volume=my_vol, + ) + + def test_reactant_setter_raises_error_with_wrong_type(): """Test a type error is raised when the first reactant is given a wrong type.""" with pytest.raises( From 6f5fb29fdfc7ad7f861fbcb927207ecebf3ff1ca Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 23:11:57 +0100 Subject: [PATCH 31/38] Checking E_p and p_0 are zero with no products --- festim/reaction.py | 25 +++++++++++++++++++++---- test/test_reaction.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/festim/reaction.py b/festim/reaction.py index 9ff91562a..2af89277e 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -55,12 +55,12 @@ def __init__( E_p: float, volume: F.VolumeSubdomain1D, ) -> None: - self.reactant = reactant - self.product = product or [] self.k_0 = k_0 self.E_k = E_k - self.p_0 = p_0 - self.E_p = E_p + self.p_0 = p_0 or 0.0 + self.E_p = E_p or 0.0 + self.reactant = reactant + self.product = product or [] self.volume = volume @property @@ -82,6 +82,23 @@ def reactant(self, value): ) self._reactant = value + @property + def product(self): + return self._product + + @product.setter + def product(self, value): + if value == []: + if self.p_0 != 0.0: + raise ValueError( + f"p_0 must be 0, not {self.p_0} when no products are present." + ) + elif self.E_p != 0.0: + raise ValueError( + f"E_p must be 0, not {self.E_p} when no products are present." + ) + self._product = value + def __repr__(self) -> str: reactants = " + ".join([str(reactant) for reactant in self.reactant]) diff --git a/test/test_reaction.py b/test/test_reaction.py index eea9a6d77..19267751b 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -281,3 +281,35 @@ def test_reactant_setter_raises_error_with_wrong_type(): E_p=0.2, volume=my_vol, ) + + +def test_product_setter_raise_error_p_0_no_product(): + with pytest.raises( + ValueError, + match="E_p must be 0, not 2 when no products are present.", + ): + F.Reaction( + reactant=[F.Species("A")], + product=None, + k_0=1, + E_k=0.1, + p_0=2, + E_p=0, + volume=my_vol, + ) + + +def test_product_setter_raise_error_E_p_no_product(): + with pytest.raises( + ValueError, + match="E_p must be 0, not 2 when no products are present.", + ): + F.Reaction( + reactant=[F.Species("A")], + product=None, + k_0=1, + E_k=0.1, + p_0=0, + E_p=2, + volume=my_vol, + ) From f0978e38d31474c84075f4ce13981bf4f0f01759 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 23:14:59 +0100 Subject: [PATCH 32/38] Removing None from str() and repr() when no product is present. --- test/test_reaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index 19267751b..ba2452840 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -96,7 +96,7 @@ def test_reaction_repr_0_products(): ) # check that the __repr__ method returns the expected string - expected_repr = "Reaction(A <--> None, 1.0, 0.2, 0.1, 0.3)" + expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.1, 0.3)" assert repr(reaction) == expected_repr @@ -165,7 +165,7 @@ def test_reaction_str_0_products(): ) # check that the __str__ method returns the expected string - expected_str = "A <--> None" + expected_str = "A <--> " assert str(reaction) == expected_str From daa8c5ca207e017869c20615c2f0f288377f7eab Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 23:18:36 +0100 Subject: [PATCH 33/38] Fix to test --- test/test_reaction.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index ba2452840..be11ba7b1 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -248,7 +248,6 @@ def arrhenius(pre, act, T): ) assert reaction.reaction_term(temperature) == expected_reaction_term - def test_reactant_setter_raises_error_with_zero_length_list(): """Test a value error is raised when the first reactant is given a wrong type.""" with pytest.raises( @@ -265,7 +264,6 @@ def test_reactant_setter_raises_error_with_zero_length_list(): volume=my_vol, ) - def test_reactant_setter_raises_error_with_wrong_type(): """Test a type error is raised when the first reactant is given a wrong type.""" with pytest.raises( @@ -282,12 +280,11 @@ def test_reactant_setter_raises_error_with_wrong_type(): volume=my_vol, ) - def test_product_setter_raise_error_p_0_no_product(): with pytest.raises( - ValueError, - match="E_p must be 0, not 2 when no products are present.", - ): + ValueError, + match="p_0 must be 0, not 2 when no products are present.", +): F.Reaction( reactant=[F.Species("A")], product=None, @@ -298,12 +295,11 @@ def test_product_setter_raise_error_p_0_no_product(): volume=my_vol, ) - def test_product_setter_raise_error_E_p_no_product(): with pytest.raises( - ValueError, - match="E_p must be 0, not 2 when no products are present.", - ): + ValueError, + match="E_p must be 0, not 2 when no products are present.", +): F.Reaction( reactant=[F.Species("A")], product=None, From 07879fb73afd23e29797258bb77165f591c730a4 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 23:21:24 +0100 Subject: [PATCH 34/38] Test fix --- test/test_reaction.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index be11ba7b1..3ad57c591 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -90,8 +90,8 @@ def test_reaction_repr_0_products(): None, k_0=1.0, E_k=0.2, - p_0=0.1, - E_p=0.3, + p_0=0, + E_p=0, volume=my_vol, ) @@ -159,8 +159,8 @@ def test_reaction_str_0_products(): None, k_0=1.0, E_k=0.2, - p_0=0.1, - E_p=0.3, + p_0=0, + E_p=0, volume=my_vol, ) From 1c62b57ed915e425cf126708d752b53526c079d3 Mon Sep 17 00:00:00 2001 From: Allentro Date: Wed, 3 Apr 2024 23:25:48 +0100 Subject: [PATCH 35/38] Fixing tests --- test/test_reaction.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/test_reaction.py b/test/test_reaction.py index 3ad57c591..5cf1ea504 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -96,7 +96,7 @@ def test_reaction_repr_0_products(): ) # check that the __repr__ method returns the expected string - expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.1, 0.3)" + expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.0, 0.0)" assert repr(reaction) == expected_repr @@ -248,6 +248,7 @@ def arrhenius(pre, act, T): ) assert reaction.reaction_term(temperature) == expected_reaction_term + def test_reactant_setter_raises_error_with_zero_length_list(): """Test a value error is raised when the first reactant is given a wrong type.""" with pytest.raises( @@ -264,6 +265,7 @@ def test_reactant_setter_raises_error_with_zero_length_list(): volume=my_vol, ) + def test_reactant_setter_raises_error_with_wrong_type(): """Test a type error is raised when the first reactant is given a wrong type.""" with pytest.raises( @@ -280,11 +282,12 @@ def test_reactant_setter_raises_error_with_wrong_type(): volume=my_vol, ) + def test_product_setter_raise_error_p_0_no_product(): with pytest.raises( - ValueError, - match="p_0 must be 0, not 2 when no products are present.", -): + ValueError, + match="p_0 must be 0, not 2 when no products are present.", + ): F.Reaction( reactant=[F.Species("A")], product=None, @@ -295,11 +298,12 @@ def test_product_setter_raise_error_p_0_no_product(): volume=my_vol, ) + def test_product_setter_raise_error_E_p_no_product(): with pytest.raises( - ValueError, - match="E_p must be 0, not 2 when no products are present.", -): + ValueError, + match="E_p must be 0, not 2 when no products are present.", + ): F.Reaction( reactant=[F.Species("A")], product=None, From bbd1cebd933a1846c1e642946e6e322793c3d888 Mon Sep 17 00:00:00 2001 From: Allentro Date: Sun, 5 May 2024 14:05:10 +0100 Subject: [PATCH 36/38] Fixing tests --- festim/hydrogen_transport_problem.py | 5 -- festim/reaction.py | 50 ++++++------- test/test_reaction.py | 103 +++++++++++++++++++-------- 3 files changed, 99 insertions(+), 59 deletions(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 925fa3707..971170b40 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -655,11 +655,6 @@ def create_formulation(self): self.formulation += ((u - u_n) / self.dt) * v * self.dx(vol.id) for reaction in self.reactions: - # reactant - if isinstance(reaction.reactant, list): - reactants = reaction.reactant - else: - reactants = [reaction.reactant] for reactant in reactants: if isinstance(reactant, F.Species): self.formulation += ( diff --git a/festim/reaction.py b/festim/reaction.py index 2af89277e..482592888 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -48,19 +48,19 @@ def __init__( reactant: Union[ F.Species, F.ImplicitSpecies, List[Union[F.Species, F.ImplicitSpecies]] ], - product: Optional[Union[F.Species, List[F.Species]]], k_0: float, E_k: float, - p_0: float, - E_p: float, volume: F.VolumeSubdomain1D, + product: Optional[Union[F.Species, List[F.Species]]] = [], + p_0: float = None, + E_p: float = None, ) -> None: self.k_0 = k_0 self.E_k = E_k - self.p_0 = p_0 or 0.0 - self.E_p = E_p or 0.0 + self.p_0 = p_0 + self.E_p = E_p self.reactant = reactant - self.product = product or [] + self.product = product self.volume = volume @property @@ -82,23 +82,6 @@ def reactant(self, value): ) self._reactant = value - @property - def product(self): - return self._product - - @product.setter - def product(self, value): - if value == []: - if self.p_0 != 0.0: - raise ValueError( - f"p_0 must be 0, not {self.p_0} when no products are present." - ) - elif self.E_p != 0.0: - raise ValueError( - f"E_p must be 0, not {self.E_p} when no products are present." - ) - self._product = value - def __repr__(self) -> str: reactants = " + ".join([str(reactant) for reactant in self.reactant]) @@ -122,8 +105,25 @@ def reaction_term(self, temperature): Arguments: temperature (): The temperature at which the reaction term is computed. """ + + if self.product == []: + if self.p_0 is not None: + raise ValueError( + f"p_0 must be None, not {self.p_0} when no products are present." + ) + if self.E_p is not None: + raise ValueError( + f"E_p must be None, not {self.E_p} when no products are present." + ) + k = self.k_0 * exp(-self.E_k / (F.k_B * temperature)) - p = self.p_0 * exp(-self.E_p / (F.k_B * temperature)) + + if self.p_0 and self.E_p: + p = self.p_0 * exp(-self.E_p / (F.k_B * temperature)) + elif self.p_0: + p = self.p_0 + else: + p = 0 reactants = self.reactant product_of_reactants = reactants[0].concentration @@ -141,4 +141,4 @@ def reaction_term(self, temperature): product_of_products *= product.solution else: product_of_products = 0 - return k * product_of_reactants - p * product_of_products + return k * product_of_reactants - (p * product_of_products) diff --git a/test/test_reaction.py b/test/test_reaction.py index 5cf1ea504..b2960c8ad 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -19,7 +19,13 @@ def test_reaction_init(): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + reactant=[species1, species2], + product=product, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, ) # check that the attributes are set correctly @@ -43,7 +49,13 @@ def test_reaction_repr(): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + reactant=[species1, species2], + product=product, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, ) # check that the __repr__ method returns the expected string @@ -64,8 +76,8 @@ def test_reaction_repr_2_products(): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], - [product1, product2], + reactant=[species1, species2], + product=[product1, product2], k_0=1.0, E_k=0.2, p_0=0.1, @@ -86,17 +98,14 @@ def test_reaction_repr_0_products(): # create a reaction between the two species reaction = F.Reaction( - species1, - None, + reactant=species1, k_0=1.0, E_k=0.2, - p_0=0, - E_p=0, volume=my_vol, ) # check that the __repr__ method returns the expected string - expected_repr = "Reaction(A <--> , 1.0, 0.2, 0.0, 0.0)" + expected_repr = "Reaction(A <--> , 1.0, 0.2, None, None)" assert repr(reaction) == expected_repr @@ -112,7 +121,13 @@ def test_reaction_str(): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + reactant=[species1, species2], + product=product, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, ) # check that the __str__ method returns the expected string @@ -133,8 +148,8 @@ def test_reaction_str_2_products(): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], - [product1, product2], + reactant=[species1, species2], + product=[product1, product2], k_0=1.0, E_k=0.2, p_0=0.1, @@ -147,7 +162,7 @@ def test_reaction_str_2_products(): assert str(reaction) == expected_str -def test_reaction_str_0_products(): +def test_reaction_str_no_products(): """Test that the Reaction __str__ method returns the expected string when there are 2 products""" # create two species @@ -155,12 +170,9 @@ def test_reaction_str_0_products(): # create a reaction between the two species reaction = F.Reaction( - species1, - None, + reactant=species1, k_0=1.0, E_k=0.2, - p_0=0, - E_p=0, volume=my_vol, ) @@ -188,7 +200,13 @@ def test_reaction_reaction_term(temperature): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], product, k_0=1.0, E_k=0.2, p_0=0.1, E_p=0.3, volume=my_vol + reactant=[species1, species2], + product=product, + k_0=1.0, + E_k=0.2, + p_0=0.1, + E_p=0.3, + volume=my_vol, ) # test the reaction term at a given temperature @@ -205,6 +223,36 @@ def arrhenius(pre, act, T): assert reaction.reaction_term(temperature) == expected_reaction_term +@pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) +def test_reaction_reaction_term_no_products(temperature): + mesh = create_unit_cube(MPI.COMM_WORLD, 10, 10, 10) + V = functionspace(mesh, ("Lagrange", 1)) + + # create two species + species1 = F.Species("A") + species2 = F.Species("B") + species1.solution = Function(V) + species2.solution = Function(V) + + # create a reaction between the two species + reaction = F.Reaction( + reactant=[species1, species2], + k_0=1.0, + E_k=0.2, + volume=my_vol, + ) + + # test the reaction term at a given temperature + def arrhenius(pre, act, T): + return pre * exp(-act / (F.k_B * T)) + + k = arrhenius(reaction.k_0, reaction.E_k, temperature) + + expected_reaction_term = k * (species1.solution * species2.solution) + + assert reaction.reaction_term(temperature) == expected_reaction_term + + @pytest.mark.parametrize("temperature", [300.0, 350, 370, 500.0]) def test_reaction_reaction_term_2_products(temperature): """Test that the Reaction.reaction_term method returns the expected reaction term with two products""" @@ -226,8 +274,8 @@ def test_reaction_reaction_term_2_products(temperature): # create a reaction between the two species reaction = F.Reaction( - [species1, species2], - [product1, product2], + reactant=[species1, species2], + product=[product1, product2], k_0=1.0, E_k=0.2, p_0=0.1, @@ -257,7 +305,6 @@ def test_reactant_setter_raises_error_with_zero_length_list(): ): F.Reaction( reactant=[], - product=None, k_0=1, E_k=0.1, p_0=2, @@ -286,30 +333,28 @@ def test_reactant_setter_raises_error_with_wrong_type(): def test_product_setter_raise_error_p_0_no_product(): with pytest.raises( ValueError, - match="p_0 must be 0, not 2 when no products are present.", + match="p_0 must be None, not 2 when no products are present.", ): - F.Reaction( + reaction = F.Reaction( reactant=[F.Species("A")], - product=None, k_0=1, E_k=0.1, p_0=2, - E_p=0, volume=my_vol, ) + reaction.reaction_term(temperature=500) def test_product_setter_raise_error_E_p_no_product(): with pytest.raises( ValueError, - match="E_p must be 0, not 2 when no products are present.", + match="E_p must be None, not 2 when no products are present.", ): - F.Reaction( + reaction = F.Reaction( reactant=[F.Species("A")], - product=None, k_0=1, E_k=0.1, - p_0=0, E_p=2, volume=my_vol, ) + reaction.reaction_term(temperature=500) From 5caf2728dde95d40dc5f36c912f16d5ea9e9fb03 Mon Sep 17 00:00:00 2001 From: Allentro Date: Sun, 5 May 2024 14:12:34 +0100 Subject: [PATCH 37/38] Fixing reactant --- festim/hydrogen_transport_problem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 971170b40..5603261f6 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -655,7 +655,7 @@ def create_formulation(self): self.formulation += ((u - u_n) / self.dt) * v * self.dx(vol.id) for reaction in self.reactions: - for reactant in reactants: + for reactant in reaction.reactant: if isinstance(reactant, F.Species): self.formulation += ( reaction.reaction_term(self.temperature_fenics) From f8624811866843bb80b5e64844f16f745bee490d Mon Sep 17 00:00:00 2001 From: Allentro Date: Sun, 5 May 2024 14:39:30 +0100 Subject: [PATCH 38/38] Adding reaction product tests --- festim/reaction.py | 9 +++++++++ test/test_reaction.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/festim/reaction.py b/festim/reaction.py index 482592888..08b25ce8e 100644 --- a/festim/reaction.py +++ b/festim/reaction.py @@ -115,6 +115,15 @@ def reaction_term(self, temperature): raise ValueError( f"E_p must be None, not {self.E_p} when no products are present." ) + else: + if self.p_0 == None: + raise ValueError( + f"p_0 cannot be None when reaction products are present." + ) + elif self.E_p == None: + raise ValueError( + f"E_p cannot be None when reaction products are present." + ) k = self.k_0 * exp(-self.E_k / (F.k_B * temperature)) diff --git a/test/test_reaction.py b/test/test_reaction.py index b2960c8ad..e1dc47617 100644 --- a/test/test_reaction.py +++ b/test/test_reaction.py @@ -345,6 +345,38 @@ def test_product_setter_raise_error_p_0_no_product(): reaction.reaction_term(temperature=500) +def test_no_E_p_with_product(): + with pytest.raises( + ValueError, + match="E_p cannot be None when reaction products are present.", + ): + reaction = F.Reaction( + reactant=[F.Species("A")], + product=[F.Species("C")], + k_0=1, + E_k=0.1, + p_0=0.1, + volume=my_vol, + ) + reaction.reaction_term(temperature=500) + + +def test_no_p_0_with_product(): + with pytest.raises( + ValueError, + match="p_0 cannot be None when reaction products are present.", + ): + reaction = F.Reaction( + reactant=[F.Species("A")], + product=[F.Species("C")], + k_0=1, + E_k=0.1, + E_p=1, + volume=my_vol, + ) + reaction.reaction_term(temperature=500) + + def test_product_setter_raise_error_E_p_no_product(): with pytest.raises( ValueError,