-
Notifications
You must be signed in to change notification settings - Fork 43
new OutflowBC #1081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
new OutflowBC #1081
Changes from 13 commits
df074f4
4924f58
b8027f7
b24ac85
be69dbf
bf3d037
7dad4ed
a11f25e
703cfa1
55c9082
7f7580f
c2efd18
6d1929a
fb03566
2686fbc
708a3a7
3d26353
64b4deb
0224269
fdffddf
3cd058e
f1ef599
ebbf54b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import ufl | ||
| from dolfinx import fem | ||
|
|
||
| from festim import subdomain as _subdomain | ||
| from festim.advection import VelocityField | ||
| from festim.species import Species | ||
|
|
||
|
|
||
| class OutflowBC: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this inherit from
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm maybe not, honestly it looks like a |
||
| """ | ||
| Dirichlet boundary condition class | ||
| u = value | ||
|
jhdark marked this conversation as resolved.
Outdated
|
||
|
|
||
| Args: | ||
| subdomain: The surface subdomain where the boundary condition is applied | ||
| value: The value of the boundary condition | ||
|
|
||
| Attributes: | ||
| subdomain: The surface subdomain where the boundary condition is applied | ||
| value: The value of the boundary condition | ||
| value_fenics: The value of the boundary condition in fenics format | ||
| bc_expr: The expression of the boundary condition that is used to | ||
| update the `value_fenics` | ||
|
jhdark marked this conversation as resolved.
Outdated
|
||
|
|
||
| """ | ||
|
|
||
| subdomain: _subdomain.SurfaceSubdomain | ||
|
jhdark marked this conversation as resolved.
|
||
|
|
||
| def __init__( | ||
| self, | ||
| velocity: fem.Function, | ||
| species: Species | list[Species], | ||
| subdomain: _subdomain.SurfaceSubdomain, | ||
| ): | ||
| self.subdomain = subdomain | ||
| self.velocity = velocity | ||
| self.species = species | ||
|
|
||
| @property | ||
| def velocity(self): | ||
| return self._velocity | ||
|
|
||
| @velocity.setter | ||
| def velocity(self, value): | ||
| err_message = f"velocity must be a fem.Function, or callable not {type(value)}" | ||
| if value is None: | ||
| self._velocity = VelocityField(value) | ||
| elif isinstance( | ||
| value, | ||
| fem.Function, | ||
| ): | ||
| self._velocity = VelocityField(value) | ||
| elif isinstance(value, fem.Constant | fem.Expression | ufl.core.expr.Expr): | ||
| raise TypeError(err_message) | ||
| elif callable(value): | ||
| self._velocity = VelocityField(value) | ||
| else: | ||
| raise TypeError(err_message) | ||
|
|
||
| @property | ||
| def species(self) -> list[Species]: | ||
| return self._species | ||
|
|
||
| @species.setter | ||
| def species(self, value): | ||
| if not isinstance(value, list): | ||
| value = [value] | ||
| # check that all species are of type festim.Species | ||
| for spe in value: | ||
| if not isinstance(spe, Species): | ||
| raise TypeError( | ||
| f"elements of species must be of type festim.Species not " | ||
| f"{type(spe)}" | ||
| ) | ||
| self._species = value | ||
|
|
||
| @property | ||
| def subdomain(self): | ||
| return self._subdomain | ||
|
|
||
| @subdomain.setter | ||
| def subdomain(self, value): | ||
| if value is None: | ||
| self._subdomain = value | ||
| elif isinstance(value, _subdomain.SurfaceSubdomain): | ||
| self._subdomain = value | ||
| else: | ||
| raise TypeError( | ||
| f"Subdomain must be a festim.SurfaceSubdomain object, not {type(value)}" | ||
| ) | ||
|
|
||
| @property | ||
| def time_dependent(self): | ||
| if self.velocity is None: | ||
| raise TypeError("Value must be given to determine if its time dependent") | ||
| if isinstance(self.velocity, fem.Constant): | ||
| return False | ||
| if callable(self.velocity): | ||
| arguments = self.velocity.__code__.co_varnames | ||
| return "t" in arguments | ||
| else: | ||
| return False | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import ufl | ||
| from dolfinx import fem | ||
| from scifem import assemble_scalar | ||
|
|
||
| from festim.exports.surface_flux import SurfaceFlux | ||
| from festim.species import Species | ||
| from festim.subdomain.surface_subdomain import SurfaceSubdomain | ||
|
|
||
|
|
||
| class Advection_flux(SurfaceFlux): | ||
|
jhdark marked this conversation as resolved.
Outdated
|
||
| """Computes the advective flux of a field on a given surface | ||
|
RemDelaporteMathurin marked this conversation as resolved.
|
||
|
|
||
| Args: | ||
| field: species for which the advective flux is computed | ||
| surface: surface subdomain | ||
| filename: name of the file to which the advective flux is | ||
| exported | ||
| velocity_field: velocity field for which the advective flux is computed | ||
|
jhdark marked this conversation as resolved.
Outdated
|
||
|
|
||
| Attributes: | ||
| see `festim.SurfaceFlux` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well there's also |
||
| """ | ||
|
|
||
| field: Species | ||
| surface: SurfaceSubdomain | ||
| velocity_field: fem.Function | ||
|
|
||
| def __init__( | ||
| self, | ||
| field: Species, | ||
| surface: SurfaceSubdomain, | ||
| velocity_field: fem.Function, | ||
| filename: str | None = None, | ||
| ): | ||
| super().__init__(field=field, surface=surface, filename=filename) | ||
|
|
||
| self.velocity_field = velocity_field | ||
|
|
||
| @property | ||
| def title(self): | ||
| return f"{self.field.name} advective flux surface {self.surface.id}" | ||
|
|
||
| def compute(self, u, ds: ufl.Measure, entity_maps=None): | ||
| """Computes the value of the flux at the surface | ||
|
|
||
| Args: | ||
| ds (ufl.Measure): surface measure of the model | ||
|
Comment on lines
+55
to
+56
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please update |
||
| """ | ||
|
|
||
| mesh = ds.ufl_domain() | ||
| n = ufl.FacetNormal(mesh) | ||
|
|
||
| surface_flux = assemble_scalar( | ||
| fem.form( | ||
| -self.D * ufl.dot(ufl.grad(u), n) * ds(self.surface.id), | ||
| entity_maps=entity_maps, | ||
| ) | ||
| ) | ||
| advective_flux = assemble_scalar( | ||
| fem.form( | ||
| u * ufl.inner(self.velocity_field, n) * ds(self.surface.id), | ||
| entity_maps=entity_maps, | ||
| ) | ||
| ) | ||
|
|
||
| self.value = surface_flux + advective_flux | ||
| self.data.append(self.value) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -754,6 +754,12 @@ def convert_advection_term_to_fenics_objects(self): | |
| function_space=self.function_space, t=self.t | ||
| ) | ||
|
|
||
| for bc in self.boundary_conditions: | ||
| if isinstance(bc, boundary_conditions.OutflowBC): | ||
| bc.velocity.convert_input_value( | ||
| function_space=self.function_space, t=self.t | ||
| ) | ||
|
|
||
| def create_flux_values_fenics(self): | ||
| """For each particle flux create the value_fenics""" | ||
| for bc in self.boundary_conditions: | ||
|
|
@@ -882,15 +888,28 @@ def create_formulation(self): | |
| * self.ds(flux_bc.subdomain.id) | ||
| ) | ||
|
|
||
| for adv_term in self.advection_terms: | ||
| # create vector functionspace based on the elements in the mesh | ||
| # add outflow term for outflow bcs in advection diffusion cases | ||
| if isinstance(bc, boundary_conditions.OutflowBC): | ||
| for species in bc.species: | ||
| conc = species.solution | ||
| v = species.test_function | ||
| vel = bc.velocity.fenics_object | ||
|
|
||
| outflow_term = ( | ||
| ufl.inner(vel * conc, self.mesh.n) | ||
| * v | ||
| * self.ds(bc.subdomain.id) | ||
| ) | ||
| self.formulation += outflow_term | ||
|
Comment on lines
+903
to
+915
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comparing this to the code above, for
|
||
|
|
||
| # add advection terms | ||
| for adv_term in self.advection_terms: | ||
| for species in adv_term.species: | ||
| conc = species.solution | ||
| v = species.test_function | ||
| vel = adv_term.velocity.fenics_object | ||
|
|
||
| advection_term = ufl.inner(ufl.dot(ufl.grad(conc), vel), v) * self.dx( | ||
| advection_term = -ufl.inner(vel * conc, ufl.grad(v)) * self.dx( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does that mean #1080 is fixed by this PR? |
||
| adv_term.subdomain.id | ||
| ) | ||
| self.formulation += advection_term | ||
|
|
@@ -1011,11 +1030,6 @@ def post_processing(self): | |
| export, | ||
| exports.SurfaceFlux | exports.TotalSurface | exports.AverageSurface, | ||
| ): | ||
| if len(self.advection_terms) > 0: | ||
| warnings.warn( | ||
| "Advection terms are not currently accounted for in the " | ||
| "evaluation of surface flux values" | ||
| ) | ||
|
Comment on lines
-1033
to
-1037
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think instead of removing this we should still have a warning if someone tries to compute |
||
| export.compute(export.field.solution, self.ds) | ||
| else: | ||
| export.compute() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -279,3 +279,82 @@ def velocity_func(x): | |||
|
|
||||
| my_model.initialise() | ||||
| my_model.run() | ||||
|
|
||||
|
|
||||
| def test_flux_conservation_advection(): | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a verification test that goes in the V&V book, ping @hhy2022 |
||||
| """MMS coupled heat and hydrogen test with 1 mobile species and 1 trap in a 1s | ||||
| transient, the values of the temperature, mobile and trapped solutions at the last | ||||
| time step is compared to an analytical solution""" | ||||
|
|
||||
| my_model = F.HydrogenTransportProblem() | ||||
|
|
||||
| test_mesh_2d = create_unit_square(MPI.COMM_WORLD, 200, 200) | ||||
|
jhdark marked this conversation as resolved.
Outdated
|
||||
| my_model.mesh = F.Mesh(test_mesh_2d) | ||||
|
|
||||
| # common festim objects | ||||
| test_mat = F.Material(D_0=1.2, E_D=0.1) | ||||
| test_vol_sub = F.VolumeSubdomain(id=1, material=test_mat) | ||||
| left = F.SurfaceSubdomain(id=1, locator=lambda x: np.isclose(x[0], 0)) | ||||
| right = F.SurfaceSubdomain(id=2, locator=lambda x: np.isclose(x[0], 1)) | ||||
| bottom = F.SurfaceSubdomain(id=3, locator=lambda x: np.isclose(x[1], 0)) | ||||
| top = F.SurfaceSubdomain(id=4, locator=lambda x: np.isclose(x[1], 1)) | ||||
| my_model.subdomains = [test_vol_sub, left, right, bottom, top] | ||||
|
|
||||
| test_H = F.Species("H", mobile=True) | ||||
| my_model.species = [test_H] | ||||
|
|
||||
| V = dolfinx.fem.functionspace(test_mesh_2d, ("Lagrange", 1, (2,))) | ||||
| velocity_field = dolfinx.fem.Function(V) | ||||
| factor = 100 | ||||
| velocity_field.interpolate(lambda x: (factor * np.sin(x[0]), factor * np.cos(x[1]))) | ||||
| my_model.advection_terms = [ | ||||
| F.AdvectionTerm(velocity=velocity_field, subdomain=test_vol_sub, species=test_H) | ||||
| ] | ||||
|
|
||||
| my_model.temperature = 100 | ||||
|
|
||||
| my_model.boundary_conditions = [ | ||||
| F.OutflowBC(velocity=velocity_field, species=test_H, subdomain=right), | ||||
| F.OutflowBC(velocity=velocity_field, species=test_H, subdomain=top), | ||||
| F.OutflowBC(velocity=velocity_field, species=test_H, subdomain=left), | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
not needed
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. although we can keep it for completeness that way we can give an arbitrary velocity field, up to you |
||||
| F.FixedConcentrationBC(species=test_H, subdomain=bottom, value=0), | ||||
| ] | ||||
|
|
||||
| my_model.sources = [F.ParticleSource(species=test_H, value=1, volume=test_vol_sub)] | ||||
|
|
||||
| my_model.settings = F.Settings(transient=False, atol=1e-10, rtol=1e-10) | ||||
|
|
||||
| advection_flux_top = F.Advection_flux( | ||||
| velocity_field=velocity_field, field=test_H, surface=top | ||||
| ) | ||||
| advection_flux_right = F.Advection_flux( | ||||
| velocity_field=velocity_field, field=test_H, surface=right | ||||
| ) | ||||
| advection_flux_left = F.Advection_flux( | ||||
| velocity_field=velocity_field, field=test_H, surface=left | ||||
| ) | ||||
| advection_flux_bottom = F.Advection_flux( | ||||
| velocity_field=velocity_field, field=test_H, surface=bottom | ||||
| ) | ||||
| my_model.exports = [ | ||||
| advection_flux_top, | ||||
| advection_flux_right, | ||||
| advection_flux_left, | ||||
| advection_flux_bottom, | ||||
| ] | ||||
|
|
||||
| my_model.initialise() | ||||
| my_model.run() | ||||
|
|
||||
| total_source = dolfinx.fem.assemble_scalar( | ||||
| dolfinx.fem.form(my_model.sources[0].value.fenics_object * ufl.dx) | ||||
| ) | ||||
|
|
||||
| total_outflux = ( | ||||
| advection_flux_top.data[-1] | ||||
| + advection_flux_right.data[-1] | ||||
| + advection_flux_left.data[-1] | ||||
| + advection_flux_bottom.data[-1] | ||||
| ) | ||||
|
|
||||
| assert np.isclose(total_source, total_outflux) | ||||
|
jhdark marked this conversation as resolved.
Outdated
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't see a ValueError anywhere here