Total lagrangian #965
Total lagrangian #965
Conversation
|
Okay, I will squash and update tests when I get feedback from the tester, but I think I'm ready for some review on this. |
|
/run-tests |
| @@ -1202,7 +1202,16 @@ namespace aspect | |||
| MeshRefinement::Manager<dim> mesh_refinement_manager; | |||
| HeatingModel::Manager<dim> heating_model_manager; | |||
|
|
|||
| const MappingQ<dim> mapping; | |||
| //Pointer, since we can have different types of mapping than MappingQ | |||
|
can we move the freesurface class into a separate header? |
| //a MappingQ1Eulerian, then we need to destroy | ||
| //the mapping before the Vector it is pointing to | ||
| //is destroyed. | ||
| mapping.reset(); |
There was a problem hiding this comment.
can you do this in the destructor of FreeSurfaceHandler, or is that too late?
There was a problem hiding this comment.
I almost did that, but I thought it was slightly cleaner here, since the Simulator is technically the owner of this object.
There was a problem hiding this comment.
I think that's the wrong logic. The simulator already deletes it, by virtue of this being a smart pointer. If you need that to happen earlier, then the class that needs it to happen earlier should be responsible.
|
Yeah, I think it is probably worth moving it to a separate header file. I will do that. |
|
This PR will change the notion of GeometryModel::depth() to be the the "reference domain depth", right? Is this acceptable? |
|
You didn't change any of the logic involved with determining the direction to use for advecting the surface vertices. Is this still identical because the surface normal is now given by the new mapping? |
|
I was talking about geometry_model->depth() with Menno. I don't think that this actually changes the behavior. In basically all of the geometry models, the depth calculation is relative to some idealized geometry model and does not rely on the triangulation at all. |
|
Correct. Since the surface normal is computed by an FEFaceValues object which takes the correct mapping, it should still be the correct normal. |
|
Looks good otherwise, but I would like @bangerth to have a look too. |
ian-r-rose
force-pushed
the
total_lagrangian
branch
2 times, most recently
from
7e535c6
to
1ee96ca
Compare
|
Okay, I have addressed the comments. Care to take another pass while I wait for (new) test diffs? |
|
This should be close to ready. |
|
|
||
| /** | ||
| * Finite element for the free surface implementation, which is | ||
| used for tracking mesh deformation. |
There was a problem hiding this comment.
can you fix the indentation and lack of asterisk at the beginning of the second line?
| //surface active, since the mapping must be in place before applying boundary | ||
| //conditions that rely on it (such as no flux bcs). | ||
| if (parameters.free_surface_enabled) | ||
| free_surface->setup_dofs(); |
There was a problem hiding this comment.
You may even be able to do this in the constructor of FreeSurfaceHandler already, or does it rely on something that's not yet available at that time?
There was a problem hiding this comment.
But this would still need to be called in Simulator::setup_dofs(), since we need to redistribute the mesh degrees of freedom upon mesh refinement.
There was a problem hiding this comment.
I see. Leave it as is then.
| * element space, which is, in general, not the same finite element | ||
| * space as the Stokes system. | ||
| */ | ||
| LinearAlgebra::Vector fs_mesh_velocity; |
There was a problem hiding this comment.
Do you actually still need this as a member variable? It would seem to me that this is now just a temporary variable in the function that computes the mesh velocity, but that a multiple of it is then added to mesh_displacement. If so, I'd prefer if it were made a variable in that function, so that one is not tempted to use it elsewhere (which at this point would almost certainly be a mistake, since we don't want to use it any more.)
There was a problem hiding this comment.
This is used in interpolate_mesh_velocity(), which interpolates fs_mesh_velocity, which exists in the free surface finite element space onto mesh_velocity, which exists in the Stokes velocity space, and is subsequently used for the ALE correction. So I think I need to keep it around, at least given how it is currently structured.
There was a problem hiding this comment.
I see. Can you update the comment in front with basically what you just said? (I.e., state where the variable is used and how?)
|
Nice work, I like it a lot! Please address the remaining comments and rebase, and then we can merge. |
…a free surface instead of moving the triangulation vertices. It compiles. Update topography postprocessor, indent. Remove debug code. Add some documentation. Astyle. Remove unused mesh_position vector, since we can do the whole thing with displacements. Hopefully fix mesh constraints bug. Fix error with the order of object destruction. Compute mesh velocity more directly. Address some comments.
ian-r-rose
force-pushed
the
total_lagrangian
branch
from
7f844c4
to
b40618e
Compare
This is the initial implementation of using a MappingQ1Eulerian to describe mesh deformation. The result of this is that the mesh geometry is no longer deformed by moving vertices around, but is instead described by a vector of the displacements from the reference configuration. I wonder what the tester thinks of it.