Update adapter and partitioned-heat-conduction #1
Conversation
|
I removed the parallelization, the support for PointSources and for nearest projection from this PR. I also fixed the tests that are run via As a next step, we should get the partitioned heat equation running. Ideally: Implement more tests, if the partitioned heat equation is still failing. |
|
I get the following error in the Neumann participant: @BenjaminRodenberg do you have an idea what's the reason that? Furthermore, the coupling expression is not supported in the Dirichlet participant in DirichletBC, it raises a NotImplementedError because of a wrong dtype. |
Looks like an
I understand that this means that there is a significant difference how a In the original If dolfinx' |
| F = u * v / dt * dx + dot(grad(u), grad(v)) * dx - (u_n / dt + f_function) * v * dx | ||
|
|
||
| dofs_remaining = locate_dofs_geometrical(V, remaining_boundary) | ||
| bcs = [DirichletBC(u_D_function, dofs_remaining)] |
There was a problem hiding this comment.
Is this way of defining a DirichletBC allowed? If yes: Maybe we could somehow translate the coupling_expression below to something that "looks" like u_D_function to get it working?
There was a problem hiding this comment.
Yes, it is. Usually they are set with Function objects, see e.g. https://jorgensd.github.io/dolfinx-tutorial/chapter2/heat_code.html
It seems like the DirichletBC looks for coupling_expression.x.array, which should by a numpy array (see https://github.com/FEniCS/dolfinx/blob/e0c3206281d71e0391482f4e07e3e30cc2cc1199/python/dolfinx/fem/dirichletbc.py#L137 ).
Partitioned heat conduction seems to be working now, at least there are no more errors. However, there's still a lot that's commented out because it hasn't been adapted to dolfinx yet. The latest version of dolfinx is required to run the case!
|
I updated the adapter and the example, the output looks good to me. However, the error computation is still missing.
Note that the latest dolfinx version is required (I'm using the nemocrys/dolfinx-openfoam container based on dolfinx commit 358470227b). |
|
@BenjaminRodenberg what do you think about the current state? Can you approve that the results are correct? |
@arvedes: I did a very quick review - just from the user perspective: Unit testsI tried to run the tests, I see that most test are failing: I guess the reason is that we did not really bother with the tests (yet). I think we should fix this before merging this PR. The testing might be a bit confusing, if you need help here, I'm available. TutorialI also tried to run the partitioned heat equation, but got an error: I remember that this worked before. Is this maybe related to an update on the FEniCS-X side? Which version am I supposed to use for testing? We should also document the compatible version for the adapter (maybe even check that the right version is used when installing the adapter. See here.). Note: You can check the version via |
|
The main issue here seems to be that I took a newer version of dolfinx because of this issue. I'm using commit 358470227b in my setup. Once there is a new release we should fix the tests and document / check the version as you suggested! |
|
This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there: https://precice.discourse.group/t/precice-workshop-2022-introduce-yourself/904/30 |
|
As soon as the tests are working, we can merge this PR. Waiting for #6, because this will make it easier to run the tests on our development version. I can take care of the tests then. |
|
@BenjaminRodenberg can you have a look at the remaining failing tests? One is failing on purpose, but I'm not sure about the other one. |
|
@BenjaminRodenberg I had to remove the assert statement that we added when we looked at the code together - it would not work for the case with vector valued data. I could resolve the SIGSEGV - it was due to wrong usage of the coupling expression. As it is now a derived of There is only one failing test remaining, it's the one with the 'not tested' error. Do you want to fix that before merging? |
|
I had to use a very dirty fix in adapter_core.convert_fenicsx_to_precice(...) because I didn't find a proper way to evaluate the function at the mesh coordinates. Do you have a better idea how to do that? What do you think about using the dofs throughout the adapter? I think it'd be the natural way of coupling in dolfinx and it would simplify the code a lot. |
|
@arvedes @BenjaminRodenberg How close is this PR to completion ? I can give a hand here.
Are all the elements supported elements where a DOF always matches a node value ? I read that no C1 (Hermite) elements were supported (which is good for us I imagine), but I've only ever used Lagrangian elements. Apart from that, I noticed there was a todo in the error computation upgrade of the tutorial. I got inspiration from this tutorial and rewrote it: from dolfinx.fem import Expression, form, Function, FunctionSpace, assemble_scalar
from mpi4py import MPI
from ufl import dx
import ufl
def compute_errors(u_approx, u_ref, v, total_error_tol=10 ** -4, degree_raise=3):
# Project into higher dimension space before dividing
W = FunctionSpace(u_approx.function_space.mesh, (v.ufl_element(
).family(), v.ufl_element().degree() + degree_raise))
u_approx_W = Function(W)
u_approx_W.interpolate(u_approx)
u_ref_W = Function(W)
if isinstance(u_ref, ufl.core.expr.Expr):
u_expr = Expression(u_ref, W.element.interpolation_points)
u_ref_W.interpolate(u_expr)
else:
u_ref_W.interpolate(u_ex)
# compute pointwise L2 error
error_normalized = abs(u_ref_W - u_approx_W) / u_ref_W
# Transform pointwise error into an expression sampled on v
error_expr = Expression(error_normalized, W.element.interpolation_points)
# Project into W
error_pointwise = Function(W)
error_pointwise.interpolate(error_expr)
error_pointwise.name = "error"
# Integrate to compute L2 error norm
error_form = form(error_pointwise**2 * dx)
error_total = (u_approx.function_space.mesh.comm.allreduce(
assemble_scalar(error_form), MPI.SUM))**0.5
assert (error_total < total_error_tol)
return error_total, error_pointwiseIt's a bit more involved but it should do the trick. (I don't have write access to this repo yet) |
|
This new error computation yields approximately 1e-6 as error compared to 1e-12 in FEniCS. |
error_normalized = abs(u_ref_W - u_approx_W) / u_ref_WAre you sure about the As far as the error magnitude goes, actually |
Yep. See https://github.com/precice/tutorials/blob/master/partitioned-heat-conduction/fenics/errorcomputation.py.
I'm aware that the solution should be exact, but the inaccuracy seems to come from the integration & projection into different function spaces. I'm pretty sure that if we didn't compute integrals of the error but pointwise errors on nodes we would get much less. |
|
Hi Boris, thanks for your support!
I'm really not an expert in that, unfortunately. Just by looking at the dolfin-x tutorials it seemed to be the natural way. Where do you think would the preCICE pain be?
I'd be very helpful if you could implement the partitioned-heat-conduction-complex tutorial in dolfin-x, with it we can probably find many of the remaining issues. I'm not sure how far we should go here, in my opinion we can merge this PR soon, even though there are still quite some # TODO in the code. |
Let's stick in 2D for now. Imagine you use regular (Lagrangian) triangle elements and linear cell interpolation mapping (for volume coupling:
For these we could subdivide ourself (annoying to code but quite straightforward), but how to handle, say Crouzeix-Raviart elements ? On these, DOFs live on edge centers only. For Hermite elements, some DOFs represent values of the derivatives. This would be hard to support,even without connectivity. A possibility would be to split data and data derivatives as two different things in preCICE. But I'm no expert there.
I can have a look at that tutorial :) |
But wouldn't it be possible (assuming Lagrangian Elements for now) to directly communicate the DOF-coordinates to preCICE, which does the mapping as usual, and then directly set these values at the DOFs? u_D = fem.Function(V)
u_D.interpolate(u_exact) # u_exact is a python function
tdim = domain.topology.dim
fdim = tdim - 1
domain.topology.create_connectivity(fdim, tdim)
boundary_facets = mesh.exterior_facet_indices(domain.topology)
bc = fem.dirichletbc(u_D, fem.locate_dofs_topological(V, fdim, boundary_facets))
Great!
Good idea. We should also setup a list with the remaining TODOs to limit the scope of the PR. I think most of the goals that @BenjaminRodenberg and me defined when we started working on that are already implemented. |
I would suggest sticking with 2D and also ignoring linear cell interpolation mapping for now, lets first make sure we have the already existing nearest-projection mapping related functionality working. Lets first get the adapter to work exactly as it works for FEniCS, and then proceed from there. Regarding handling of higher order elements, the fenics-adapter only defines DOFs which are coinciding with the physical vertices as part of the coupling interface. This is basically the geometry-based interface. As stated there, it is true that one would loose information by ignoring the higher order DOFs, but this is what we did back then and I think we should first get this to work and then deal with the problem of higher order elements or elements having only edge-based DOFs. This essentially means that we are restricted to linear elements to get accurate results, which is fine for the partitioned-heat-conduction tutorial. I will write another message with a list of TODOs. An idea to handle this properly would be to open issues for every major TODO and then resolve them via independent PRs to have a clean workflow. |
Good idea let's not make thinks to complicated now!
I think this was a good decision for the legacy-fenics, for the new one I've got the feeling that we can simplify things a lot by using the DOFs throughout the adapter. But let's get back to that later!
Great, thanks! |
|
I am trying to get a hang of everything that has been changed and I realized that vector data cannot be handled right? The function |
|
Is this the not-implemented-error? I think Benjamin and me never touched that. |
|
I just tried to catch up a bit on this PR. Nice that there is so much going on here and good to see @boris-martin joining the party 🎉 One important feature of the fenics-adapter that was not mentioned above (it's also poorly documented): The interpolation routine uses segregated RBF interpolation. The algorithm is described in Lindner, F., Mehl, M., & Uekermann, B. (2017). Radial basis function interpolation for black-box multi-physics simulations. and it is also availabe in preCICE. Short explanation of the algorithmYou create a least-squares fit to the data on the coupling mesh. This gives you a second-degree global polynomial (see here). Usually this will still give you some interpolation error (if you are not very lucky). To eliminate this error and create an interpolation that goes exactly through all the available data, you use an RBF interpolation of the error. The resulting interpolant is then polynomial fit + RBF interpolation of the error. Special case: partitioned heat equationWith the partitioned heat equation we are actually lucky. This means: There is no error in the polynomial fit. Remember: Our analytical solution is second degree. Therefore, our second-degree least squares fit is exact. Nice feature: We are not only exact on the DoFs, but also between the DoFs. Since our expression is a function, we now have an exact representation of the solution at the coupling interface. This should also work with higher-order elements. This is, of course, a very special case: It only works that nicely for a second degree polynomial solution. But as @IshaanDesai mentioned: This case is mainly about high accuracy and testing. Not so much about generality (however, it also works for the complex geometry case, because it is independent from the mesh). Reasons for choosing this algorithmI chose this algorithm mainly for two reasons:
Possible pitfall for FEniCS-XI'm not sure about the implementation in FEniCS-X. When I implemented this for FEniCS I relied on the fact that the expression is an object that hides the finite elements of participant A from B and vice-versa. As long as the expression represents the exact solution everything is fine. I don't know whether this is still possible for FEniCS-X. If you cannot provide a functional representation of the data, but you have to use a pointwise representation you will need some additional magic to ensure that you do not just perform a piecewise linear interpolation. A piecewise linear interpolation will break as soon as you use higher-order elements, because they need information on mesh-nodes and between mesh-nodes. Some ideas for debugging
|
I think I found the issue. Essentially it boils down to "we don't handle the vector case": the (so-called) dirty fix from @arvedes didn't take in account the vector case when building the mask, so data was cropped. I haven't fixed it yet because there are still details to work on, but I'll handle it in the coming days. By they way, speaking of the dirty fix, I had a look at the changelog of Dolfinx and it says:
This could replace the dirty fix, I'll think about it. (compute_point_values is apparently the equivalent of FEniCS's compute_vertex_values, used in the FEniCS adapter). If we project in a CG-1 space, mapping is easy: in dimension d, the i-th component at vertex n would be given by |
#14) * working on a fix for the partitioned-heat-equation tutorial * working on a fix for the partitioned-heat-equation tutorial * Trying out solutions for L2 error computation from the Discourse * Error computation in working state * Formatting * Calculate relative error instead of absolute error and clean up script * Formatting Co-authored-by: Ishaan Desai <ishaandesai@gmail.com>
In this PR we want to update the adapter and the partitioned heat conduction tutorial to FEniCSx.
Helpful: https://jorgensd.github.io/dolfinx-tutorial/chapter2/heat_equation.html