From 9c08a5b7a4fd87ddf8864528df879ba0c807d69b Mon Sep 17 00:00:00 2001 From: RemDelaporteMathurin Date: Thu, 2 Nov 2023 08:38:47 -0400 Subject: [PATCH 1/6] added two tests for a conditional time statement --- test/test_dirichlet_bc.py | 1 + test/test_h_transport_problem.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/test/test_dirichlet_bc.py b/test/test_dirichlet_bc.py index 98d9cd731..ade7b8572 100644 --- a/test/test_dirichlet_bc.py +++ b/test/test_dirichlet_bc.py @@ -226,6 +226,7 @@ def test_callable_x_only(): lambda x, t: 1.0 + x[0] + t, lambda x, t, T: 1.0 + x[0] + t + T, lambda x, t: ufl.conditional(ufl.lt(t, 1.0), 100.0 + x[0], 0.0), + lambda t: 100.0 if t < 1 else 0.0, ], ) def test_integration_with_HTransportProblem(value): diff --git a/test/test_h_transport_problem.py b/test/test_h_transport_problem.py index c5a42d3f0..8ff9099e0 100644 --- a/test/test_h_transport_problem.py +++ b/test/test_h_transport_problem.py @@ -4,6 +4,7 @@ import dolfinx.mesh from dolfinx import fem, nls import ufl +from ufl.conditional import Conditional import numpy as np import pytest @@ -77,6 +78,7 @@ def test_define_temperature_value_error_raised(): (lambda x: 1.0 + x[0], fem.Function), (lambda x, t: 1.0 + x[0] + t, fem.Function), (lambda x, t: ufl.conditional(ufl.lt(t, 1.0), 100.0 + x[0], 0.0), fem.Function), + (lambda t: 100.0 if t < 1 else 0.0, fem.Constant), ], ) def test_define_temperature(input, expected_type): From 9848d213a40eddde3022c1c5648ef23a2ca13206 Mon Sep 17 00:00:00 2001 From: RemDelaporteMathurin Date: Thu, 2 Nov 2023 08:53:07 -0400 Subject: [PATCH 2/6] added tests that catch the behaviour --- test/test_dirichlet_bc.py | 24 ++++++++++++++++++++++++ test/test_h_transport_problem.py | 21 +++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/test/test_dirichlet_bc.py b/test/test_dirichlet_bc.py index ade7b8572..e0bbeaf92 100644 --- a/test/test_dirichlet_bc.py +++ b/test/test_dirichlet_bc.py @@ -284,6 +284,30 @@ def test_integration_with_HTransportProblem(value): assert np.isclose(computed_value, expected_value) +@pytest.mark.parametrize( + "value", + [ + lambda t: ufl.conditional(ufl.lt(t, 1.0), 1, 2), + lambda t: 1 + ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + lambda t: 2 * ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + lambda t: 2 / ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + ], +) +def test_define_value_error_if_ufl_conditional_t_only(value): + """Test that a ValueError is raised when the value attribute is a callable + of t only and contains a ufl conditional""" + + subdomain = F.SurfaceSubdomain1D(1, x=1) + species = F.Species("test") + + bc = F.DirichletBC(subdomain, value, species) + + t = fem.Constant(mesh, 0.0) + + with pytest.raises(ValueError, match="wrong type for temperature"): + bc.create_value(mesh=mesh, function_space=None, temperature=None, t=t) + + def test_species_predefined(): """Test a ValueError is raised when the species defined in the boundary condition is not predefined in the model""" diff --git a/test/test_h_transport_problem.py b/test/test_h_transport_problem.py index 8ff9099e0..53b308845 100644 --- a/test/test_h_transport_problem.py +++ b/test/test_h_transport_problem.py @@ -99,6 +99,27 @@ def test_define_temperature(input, expected_type): assert isinstance(my_model.temperature_fenics, expected_type) +@pytest.mark.parametrize( + "input", + [ + lambda t: ufl.conditional(ufl.lt(t, 1.0), 1, 2), + lambda t: 1 + ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + lambda t: 2 * ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + lambda t: 2 / ufl.conditional(ufl.lt(t, 1.0), 1, 2.0), + ], +) +def test_define_temperature_error_if_ufl_conditional_t_only(input): + """Test that a ValueError is raised when the temperature attribute is a callable + of t only and contains a ufl conditional""" + my_model = F.HydrogenTransportProblem(mesh=test_mesh) + my_model.t = fem.Constant(test_mesh.mesh, 0.0) + + my_model.temperature = input + + with pytest.raises(ValueError, match="wrong type for temperature"): + my_model.define_temperature() + + def test_iterate(): """Test that the iterate method updates the solution and time correctly""" # BUILD From 93704ef3178cfb7b696c6f5167d7ad739016a525 Mon Sep 17 00:00:00 2001 From: RemDelaporteMathurin Date: Thu, 2 Nov 2023 08:53:16 -0400 Subject: [PATCH 3/6] added type check --- festim/boundary_conditions/dirichlet_bc.py | 3 +++ festim/hydrogen_transport_problem.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/festim/boundary_conditions/dirichlet_bc.py b/festim/boundary_conditions/dirichlet_bc.py index 6b31ab22e..efe4544a8 100644 --- a/festim/boundary_conditions/dirichlet_bc.py +++ b/festim/boundary_conditions/dirichlet_bc.py @@ -1,5 +1,6 @@ import festim as F import ufl +from ufl.core.operator import Operator from dolfinx import fem import numpy as np @@ -98,6 +99,8 @@ def create_value( if "t" in arguments and "x" not in arguments and "T" not in arguments: # only t is an argument + if isinstance(self.value(t=float(t)), Operator): + raise ValueError("wrong type for temperature") self.value_fenics = F.as_fenics_constant( mesh=mesh, value=self.value(t=float(t)) ) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index d21c53874..7ff3ddd46 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -3,6 +3,7 @@ from dolfinx.io import XDMFFile import basix import ufl +from ufl.core.operator import Operator from mpi4py import MPI from dolfinx.fem import Function, form, assemble_scalar from dolfinx.mesh import meshtags @@ -196,6 +197,8 @@ def define_temperature(self): elif callable(self.temperature): arguments = self.temperature.__code__.co_varnames if "t" in arguments and "x" not in arguments and "T" not in arguments: + if isinstance(self.temperature(t=float(self.t)), Operator): + raise ValueError("wrong type for temperature") # only t is an argument self.temperature_fenics = F.as_fenics_constant( mesh=self.mesh.mesh, value=self.temperature(t=float(self.t)) From 1f7bcd097ee55931955b1096140c6abe84187828 Mon Sep 17 00:00:00 2001 From: RemDelaporteMathurin Date: Thu, 2 Nov 2023 09:42:32 -0400 Subject: [PATCH 4/6] better test --- festim/boundary_conditions/dirichlet_bc.py | 6 ++++-- festim/hydrogen_transport_problem.py | 6 ++++-- test/test_dirichlet_bc.py | 4 +++- test/test_h_transport_problem.py | 4 +++- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/festim/boundary_conditions/dirichlet_bc.py b/festim/boundary_conditions/dirichlet_bc.py index efe4544a8..b1ca05361 100644 --- a/festim/boundary_conditions/dirichlet_bc.py +++ b/festim/boundary_conditions/dirichlet_bc.py @@ -99,8 +99,10 @@ def create_value( if "t" in arguments and "x" not in arguments and "T" not in arguments: # only t is an argument - if isinstance(self.value(t=float(t)), Operator): - raise ValueError("wrong type for temperature") + if not isinstance(self.value(t=float(t)), (float, int)): + raise ValueError( + f"self.value should return a float or an int, not {type(self.value(t=float(t)))} " + ) self.value_fenics = F.as_fenics_constant( mesh=mesh, value=self.value(t=float(t)) ) diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 7ff3ddd46..6017307d7 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -197,8 +197,10 @@ def define_temperature(self): elif callable(self.temperature): arguments = self.temperature.__code__.co_varnames if "t" in arguments and "x" not in arguments and "T" not in arguments: - if isinstance(self.temperature(t=float(self.t)), Operator): - raise ValueError("wrong type for temperature") + if not isinstance(self.temperature(t=float(self.t)), (float, int)): + raise ValueError( + f"self.temperature should return a float or an int, not {type(self.temperature(t=float(self.t)))} " + ) # only t is an argument self.temperature_fenics = F.as_fenics_constant( mesh=self.mesh.mesh, value=self.temperature(t=float(self.t)) diff --git a/test/test_dirichlet_bc.py b/test/test_dirichlet_bc.py index e0bbeaf92..ea17569b7 100644 --- a/test/test_dirichlet_bc.py +++ b/test/test_dirichlet_bc.py @@ -304,7 +304,9 @@ def test_define_value_error_if_ufl_conditional_t_only(value): t = fem.Constant(mesh, 0.0) - with pytest.raises(ValueError, match="wrong type for temperature"): + with pytest.raises( + ValueError, match="self.value should return a float or an int, not " + ): bc.create_value(mesh=mesh, function_space=None, temperature=None, t=t) diff --git a/test/test_h_transport_problem.py b/test/test_h_transport_problem.py index 53b308845..09680016e 100644 --- a/test/test_h_transport_problem.py +++ b/test/test_h_transport_problem.py @@ -116,7 +116,9 @@ def test_define_temperature_error_if_ufl_conditional_t_only(input): my_model.temperature = input - with pytest.raises(ValueError, match="wrong type for temperature"): + with pytest.raises( + ValueError, match="self.temperature should return a float or an int, not " + ): my_model.define_temperature() From b13bb3ad6c90da07f37ebec1ca70bdcdfbb274c5 Mon Sep 17 00:00:00 2001 From: RemDelaporteMathurin Date: Thu, 2 Nov 2023 09:44:23 -0400 Subject: [PATCH 5/6] removed unused imports --- festim/boundary_conditions/dirichlet_bc.py | 1 - festim/hydrogen_transport_problem.py | 1 - test/test_h_transport_problem.py | 1 - 3 files changed, 3 deletions(-) diff --git a/festim/boundary_conditions/dirichlet_bc.py b/festim/boundary_conditions/dirichlet_bc.py index b1ca05361..776535e24 100644 --- a/festim/boundary_conditions/dirichlet_bc.py +++ b/festim/boundary_conditions/dirichlet_bc.py @@ -1,6 +1,5 @@ import festim as F import ufl -from ufl.core.operator import Operator from dolfinx import fem import numpy as np diff --git a/festim/hydrogen_transport_problem.py b/festim/hydrogen_transport_problem.py index 6017307d7..60ff98186 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -3,7 +3,6 @@ from dolfinx.io import XDMFFile import basix import ufl -from ufl.core.operator import Operator from mpi4py import MPI from dolfinx.fem import Function, form, assemble_scalar from dolfinx.mesh import meshtags diff --git a/test/test_h_transport_problem.py b/test/test_h_transport_problem.py index 09680016e..055001b2b 100644 --- a/test/test_h_transport_problem.py +++ b/test/test_h_transport_problem.py @@ -4,7 +4,6 @@ import dolfinx.mesh from dolfinx import fem, nls import ufl -from ufl.conditional import Conditional import numpy as np import pytest From d97cf7765bcdcf8aeb96c170b01436460e317724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Delaporte-Mathurin?= <40028739+RemDelaporteMathurin@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:22:05 -0400 Subject: [PATCH 6/6] Removed temperature check Co-authored-by: James Dark <65899899+jhdark@users.noreply.github.com> --- 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 60ff98186..666f1a18e 100644 --- a/festim/hydrogen_transport_problem.py +++ b/festim/hydrogen_transport_problem.py @@ -195,7 +195,7 @@ def define_temperature(self): # if temperature is callable, process accordingly elif callable(self.temperature): arguments = self.temperature.__code__.co_varnames - if "t" in arguments and "x" not in arguments and "T" not in arguments: + if "t" in arguments and "x" not in arguments: if not isinstance(self.temperature(t=float(self.t)), (float, int)): raise ValueError( f"self.temperature should return a float or an int, not {type(self.temperature(t=float(self.t)))} "