Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expand coupling functor support in RelationshipManager system #13736

Closed
lindsayad opened this issue Jul 15, 2019 · 2 comments · Fixed by #13750
Closed

Expand coupling functor support in RelationshipManager system #13736

lindsayad opened this issue Jul 15, 2019 · 2 comments · Fixed by #13750
Assignees
@lindsayad
Labels
P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.

Comments

@lindsayad
Copy link
Member

Reason

@WilkAndy has reported that he's seeing a lot of mallocs in MatSetValues in PETSc which is indicative that we don't have the sparsity pattern correct.

Design

We already have RelationshipManagerType::COUPLING, so when that enumerator is chosen we can make sure to call DofMap::add_coupling_functor. We will also want to make sure that we make use of the information in our CouplingMatrix so that we don't end up greatly expanding our sparsity pattern in an unnecessary way.

Impact

Avoid MatSetValues mallocs which dramatically slow down simulations.
@lindsayad lindsayad added T: task An enhancement to the software. P: normal A defect affecting operation with a low possibility of significantly affects. labels Jul 15, 2019
@lindsayad lindsayad self-assigned this Jul 15, 2019
@WilkAndy
Copy link
Contributor

I'm really grateful that you're doing this, thank you Alexander,

a

@WilkAndy
Copy link
Contributor

For testing, you could use

cd moose/modules/porous_flow/test/tests/flux_limited_TVD_advection
../../../porous_flow-opt Executioner/end_time=6E-2 VectorPostprocessors/tracer/num_points=1 Mesh/nz=N Mesh/ny=N -i fltvd_3D.i

where you should see some speedup for N>10, and probably x2 speedup for N>16

@WilkAndy WilkAndy mentioned this issue Jul 16, 2019
Closed
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 16, 2019
@lindsayad
Add support for coupling functor relationship managers
5c214a6 
Refs idaholab#13736
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 17, 2019
@lindsayad
Add support for coupling functor relationship managers
b1354e3 
Refs idaholab#13736
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 17, 2019
@lindsayad
Add support for coupling functor relationship managers
a0b1c7a 
Refs idaholab#13736
@lindsayad lindsayad mentioned this issue Jul 17, 2019
Merged
lindsayad referenced this issue in lindsayad/moose Jul 17, 2019
@lindsayad
Really remove libMesh default ghosting/coupling
9c3d887 
We are now fully explicit in the ghosting and sparsity coupling
pattern that we want. We add a coupling functor for kernels and
bcs that does not do any ghosting because it has _n_levels set
to 0, but it does specify intra-element dof coupling so that the
sparsity pattern is correct.

Now we can really re-gold the geometric edge test case because
there is really no evaluable ghosting now
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 22, 2019
@lindsayad
Add CouplingFunctorCheck action
6eadaba 
This action checks to see whether we should be adding
intra-element dof-coupling by inspecting the kernel and
boundary condition warehouses. If do need intra-dof coupling
then we add a default coupling functor and reinit the
libMesh::ImplicitSystem

Refs idaholab#13736
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 23, 2019
@lindsayad
Add CouplingFunctorCheck action
e88cebb 
This action checks to see whether we should be adding
intra-element dof-coupling by inspecting the kernel and
boundary condition warehouses. If do need intra-dof coupling
then we add a default coupling functor and reinit the
libMesh::ImplicitSystem

Refs idaholab#13736
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 26, 2019
@lindsayad
Do coupling check for dgs and iks
f9cb45c 
We should also do a coupling check for objects that need
more than 0 layers of ghosting like dgkernels and interfacekernels.
This will also be very beneficial for applications like Rattlesnake
which uses complex actions to set up their simulations.
In Rattlesnake, there is often complex logic that may or may not
add dgs or iks depending on the input file. Instead of always
adding 1 layer of ghosting/coupling always, which is what is
being currently done since the addition of coupling functors,
we should just do the check for Rattlesnake and not worry about
adding relationship managers from the Rattlesnake actions. By
inspecting the warehouses after everything has been added, we
will always get the amount of coupling needed correct, no guessing
involved.

Refs idaholab#13736
@lindsayad lindsayad mentioned this issue Jul 26, 2019
Merged
lindsayad added a commit to lindsayad/moose that referenced this issue Jul 31, 2019
@lindsayad
Make sure that 1 layer coupling is actually added
4412baf 
This marks the importance of local testing. 1 layer
of sparsity was not actually getting added in a Rattlesnake
simulation that added all its dg/interface kernels through
actions. The issue was that the logic in Action::addRelationshipManager
had determined that because the added RM had an RM type of
GEOMETRIC | ALGEBRAIC | COUPLING that the RM must have been
added during the geometric attachment stages and hence did not
add it for the requested `input_rm_type = COUPLING`. Now in
our `CouplingFunctorCheckAction` I make sure that we add an
RM with rm type restricted only to coupling, as the name of
the action implies. Note that this logic presumes that the user
has added a relationship manager that provides geometric
ghosting ahead of this coupling ghosting. If not, the user will
be in for a rude awakening if they try to run with DistributedMesh.
Note that we can't try to fix this situation for them; by the time
we've added all our compute objects to the warehouse, the mesh
has been prepared and remote elements have already been deleted.

Refs idaholab#13736
lindsayad added a commit to lindsayad/moose that referenced this issue Aug 5, 2019
@lindsayad
Make sure that 1 layer coupling is actually added
6bbccc6 
This marks the importance of local testing. 1 layer
of sparsity was not actually getting added in a Rattlesnake
simulation that added all its dg/interface kernels through
actions. The issue was that the logic in Action::addRelationshipManager
had determined that because the added RM had an RM type of
GEOMETRIC | ALGEBRAIC | COUPLING that the RM must have been
added during the geometric attachment stages and hence did not
add it for the requested `input_rm_type = COUPLING`. Now in
our `CouplingFunctorCheckAction` I make sure that we add an
RM with rm type restricted only to coupling, as the name of
the action implies. Note that this logic presumes that the user
has added a relationship manager that provides geometric
ghosting ahead of this coupling ghosting. If not, the user will
be in for a rude awakening if they try to run with DistributedMesh.
Note that we can't try to fix this situation for them; by the time
we've added all our compute objects to the warehouse, the mesh
has been prepared and remote elements have already been deleted.

Refs idaholab#13736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lindsayad lindsayad

Labels
P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Coupling Relationship Managers lindsayad/moose
2 participants
@WilkAndy @lindsayad