-
Notifications
You must be signed in to change notification settings - Fork 17
fix: periodic and mpi #385
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: master
Are you sure you want to change the base?
Conversation
I Tested this PR on two cases (
|
} | ||
|
||
template <IsMesh mesh_t> | ||
auto update_subdomains_mpi([[maybe_unused]] const mesh_t& mesh, const auto& mpi_neighbourhood) |
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.
What is this function for? It seems to be doing the same thing as update_mesh_neighbour
, but all the calls to update_mesh_neighbour
are now commented.
Why are the neighbour meshes stored into a vector? They used to be stored into the neighbour
variables inside the mesh.
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.
You are right, but at this stage, we only have a CellArray
that gives us the new mesh with only the cells. We have to make the graduation of it before the creation of the new MRMesh
. This is why I created a new one.
However, we can probably make some refactoring to call the same function on both sides. I will do that in a future commit.
Description
This PR fixes a bug that was introduced earlier for the MPI usage and the periodic. The ghost cells were not set properly.
The new implementation of the
update_sub_mesh_impl
method in themr/mesh.hpp
file correctly computes all the ghost cells used during the adaptation step and the integration of the numerical scheme.We can see this by observing that we use exactly the same algorithm to add the ghost cells in cases with one domain without periodic conditions, one domain with periodic conditions, and several domains with or without periodic conditions. This makes sense, as for the subdomain or the periodic parts, we simply translate a domain with true cells and add the ghost cells to the subdomain accordingly. The procedure is the same in both cases.
In a future version, AMR mesh will be updated using the same idea.
One of the key elements of this PR is the intense use of the expand operator, which adds one or more cells in all directions of a subset. Unfortunately, the subset implementation is not optimized for this kind of operator, as many unions are involved. The subset implementation is currently being rewritten and should reduce the time required to compute the expanded subset.
Related issue
The MPI version, both with and without periodic, was broken. The graduation method was incorrect because it did not take into account all the levels that could potentially add new cells. And finally, the constraint defined in the PR #320 was not parallel. This PR fixes this issue.
How has this been tested?
To validate the MPI version of samurai and to prevent any regression, a test case is added in
demos/FiniteVolume
calledburgers_os_2d_mpi.cpp
. The main idea is to replicate the initial solution on each subdomain and to add periodic conditions. Then, we solve the Burgers equation using an OSMP scheme, checking that the mesh on each subdomain is consistent. This example can also be used to measure the weak scaling in samurai.Code of Conduct
By submitting this PR, you agree to follow our Code of Conduct