Static condensation #3883

lindsayad · 2024-06-20T05:30:50Z

Adds the ability to condense out element internal degrees of freedom. This can be used to eliminate internal degrees of freedom in high order CFEM or to eliminate internal degrees of freedom in HDG methods

Use the same mutex for both `operator<<` overloads. Refs CIVET failure https://civet.inl.gov/job/2295949/ on libMesh/libmesh#3883

moosebuild · 2024-06-26T14:34:10Z

Job Test mac on 11f7769 : invalidated by @lindsayad

moosebuild · 2024-06-26T17:37:23Z

Job Coverage on 31388ef wanted to post the following:

Coverage

	d25162	#3883 31388e
	Total	Total	+/-	New
Rate	61.87%	62.01%	+0.14%	80.50%
Hits	72020	72451	+431	487
Misses	44392	44388	-4	118

Diff coverage report

Full coverage report

Warnings

New new line coverage rate 80.50% is less than the suggested 90.0%

This comment will be updated on new commits.

lindsayad · 2024-06-26T18:01:33Z

I've opened #3891 to keep track of what all I'm planning to do with this class. I think it makes sense to address #3891 with multiple PRs as it breaks up the author and reviewer work into logical chunks

examples/vector_fe/vector_fe_ex9/run.sh

roystgnr

There may be other issues I've missed; this is a pretty massive PR, and I'm running out of pre-vacation time, and I've already hit you with a ton of complaints. ;-)

There are a few obvious utility things we could split into smaller PRs if you wanted to get those in ASAP.

roystgnr · 2024-06-27T19:02:15Z

include/base/dof_map.h

+   * Fills the vector \p di with the global degree of freedom indices
+   * for the element.  For one variable, and potentially for a
+   * non-default element p refinement level
+   */


We've got to have some documentation here on how those functors work.

include/base/dof_map.h

include/base/libmesh_common.h

include/geom/elem.h

include/numerics/static_condensation.h

src/base/dof_map.C

src/numerics/numeric_vector.C

src/numerics/static_condensation.C

lindsayad · 2024-07-01T19:34:59Z

I think what I will do is split out the utilities into their own PRs. And then I'm going to add more work that's coming from using this in the HDG example. I'm pretty sure in that example I will not be allowed to remove the pressure dofs (even though they are all "internal") from the global system, so some more work will still need to be done to handle this in the library. Given that, I'm returning this to draft for now

lindsayad · 2024-07-01T19:36:12Z

I will not be allowed to remove the pressure dofs (even though they are all "internal")

this will go along with @roystgnr's desire to use "condensed"/"uncondensed" verbiage over internal/trace

lindsayad · 2024-07-05T21:38:47Z

Alright, here is one datapoint that suggests the DofMap modifications don't impact some simulation types. This is a profile from process 0 of the updated HDG NS example run on 32 procs with 885,761 dofs. As we would hope for, all the computation is in the solve

moosebuild · 2024-08-14T17:10:47Z

Job Min gcc on e60b60c : invalidated by @lindsayad

moosebuild · 2024-08-14T17:10:51Z

Job Test 32bit on e60b60c : invalidated by @lindsayad

moosebuild · 2024-08-14T17:10:56Z

Job Test debug on e60b60c : invalidated by @lindsayad

lindsayad · 2024-08-14T20:49:40Z

finally ready for review again 😄

roystgnr · 2024-08-26T20:19:27Z

include/base/dof_map.h

@@ -1616,7 +1634,7 @@ class DofMap : public ReferenceCountedObject<DofMap>,
   * constraints or sufficiently many user constraints.
   */
  std::unique_ptr<SparsityPattern::Build> build_sparsity(const MeshBase & mesh,
-                                                         bool calculate_constrained = false) const;
+                                                         const bool calculate_constrained = false) const;


At some point we've got to make a global decision about when pass-by-value parameters should be const. On the one hand it's helpful for protection the implementation, but on the other hand it just muddies up the declaration with an implementation-specific detail that doesn't actually promise anything.

Yeah, since we don't pass const non-reference parameters like this consistently elsewhere in the library, I agree it looks a bit out of place to make this change. I could be talked into inconsistently having const parameters in the .C file based on the author's preference, but I'm not convinced that really helps catch lots of bugs either.

roystgnr · 2024-08-26T20:29:31Z

include/base/sparsity_pattern.h

+  /// If there are "spider" nodes in the mesh (i.e. a single node which
+  /// is connected to many 1D elements) and Constraints, we can end up
+  /// sorting the same set of DOFs multiple times in handle_vi_vj(),
+  /// only to find that it has no net effect on the final sparsity. In
+  /// such cases it is much faster to keep track of (element_dofs_i,
+  /// element_dofs_j) pairs which have already been handled and not
+  /// repeat the computation. We use this data structure to keep track
+  /// of hashes of sets of dofs we have already seen, thus avoiding
+  /// unnecessary calculations.


Surely this isn't necessary?

Do you not want doxygen for it? That would be inconsistent with your other comments. Currently on the doxygen page for Build:

I didn't realize that Doxygen ignored C++ comments. Could we just switch to the ASCII-art-C-style that we use e.g. for clear_full_sparsity() above?

roystgnr · 2024-08-26T20:32:45Z

include/numerics/petsc_matrix.h

@@ -83,7 +83,7 @@ class PetscMatrix final : public PetscMatrixBase<T>
   */
  explicit
  PetscMatrix (Mat m,
-               const Parallel::Communicator & comm_in);
+                  const Parallel::Communicator & comm_in);


Clean this up

roystgnr · 2024-08-26T20:43:04Z

include/numerics/static_condensation.h

+  static void uncondensed_dofs_from_scalar_dofs(std::vector<dof_id_type> & uncondensed_dofs,
+                                                const std::vector<dof_id_type> & scalar_dofs);
+
+  static void total_dofs_from_field_dof(std::vector<dof_id_type> & dofs,


Yeah, they're private, but they all need documentation. Total dofs per what? What is a field dof?

roystgnr · 2024-08-26T20:48:22Z

include/systems/system.h

+   * have the same sparsity pattern as the matrix used during
+   * solution.  When not \p System but the user wants to
+   * initialize the mayor matrix, then all the additional matrices,
+   * if existent, have to be initialized by the user, too.


What's the value of adding a matrix to the system that the system isn't going to be maintaining it during reinit() etc? Just throwing the memory management hot potato to System to catch? That doesn't seem worth inconsistent behavior (an uninitialized matrix after reinit() if we skip it) or broken behavior (we aren't skipping anything in _matrices in System::reinit(), right? what happens when we compute_sparsity() for one you didn't want us to mess with?).

I just copy-pasted the doxygen from the method above. So I don't know what to do here. It seems like you signed off on this back when it was added in #2638

The "mayor" ("main") in the comment looks like it was your own version of the brown M&Ms trick, and I missed it. But I didn't read the comment carefully enough this time either, just freaked out at imagining trying to deal with a mix of System-initialized and user/subclass-initialized sparsity patterns.

So I guess my remaining concern is just - if we have all user/subclass-initialized matrices, aren't we still doing a compute_sparsity() anyway, and are we sure that's never going to be redundant?

roystgnr · 2024-08-26T20:52:26Z

src/numerics/petsc_matrix.C

+                              const numeric_index_type n_in,
+                              const numeric_index_type m_l,
+                              const numeric_index_type n_l,
+                              const std::vector<numeric_index_type> & n_nz,
+                              const std::vector<numeric_index_type> & n_oz,
+                              const numeric_index_type blocksize_in)


Okay, my new policy is that if I see brown M&Ms backstage I'm not putting equipment or people at risk onstage.

I can understand the active objection to labeling const in the headers, but I don't understand the active complaint about marking variables that don't change as const in the implementation. If it's an objection for the method parameters, then it seems like it should be a general objection

Does Github show you something different than it shows me here? I didn't see any change having anything to do with const, just a bunch of added whitespace that broke the formatting in the file.

roystgnr · 2024-08-26T21:32:08Z

include/solvers/petsc_linear_solver.h

@@ -257,6 +258,16 @@ class PetscLinearSolver : public LinearSolver<T>
   */
  virtual LinearConvergenceReason get_converged_reason() const override;

+protected:
+#if defined(LIBMESH_ENABLE_AMR) && defined(LIBMESH_HAVE_METAPHYSICL)


Do we need these guards? I thought the wrapper was good for field splits even on an unrefined mesh, and I know MetaPhysicL is only for the refinement+GMG case.

roystgnr · 2024-08-26T21:33:15Z

include/systems/implicit_system.h

@@ -335,13 +336,37 @@ class ImplicitSystem : public ExplicitSystem
   */
  mutable std::unique_ptr<LinearSolver<Number>> linear_solver;

+  /**
+   * Retrieve the static condensation class. Errors if the user has not
+   * requested it on the command line, so calls to this should be guarded by


Surely we should have both a CLI and a C++ setter interface to that?

src/solvers/petsc_linear_solver.C

roystgnr · 2024-08-26T21:44:50Z

src/solvers/tao_optimization_solver.C

@@ -398,7 +398,7 @@ extern "C"
    sys.update();
    X.swap(X_sys);

-    // Let's also wrap J and Jpre in PetscMatrix objects for convenience
+    // Let's also wrap J and Jpre in PetscMatrixBase objects for convenience


Not sure if I missed these last time or if you got overzealous search-and-replacing comments to fix a converse complaint of mine last time?

roystgnr · 2024-08-26T21:46:34Z

src/systems/implicit_system.C

 {
+  if (libMesh::on_command_line("--" + name_in + "-static-condensation"))


Can we move this block to a separate method so static condensation can be enabled from code? Or would it have to be a constructor argument? It would seem to me that turning SC on any time before System::init() ought to be feasible, at least.

Use the same mutex for both `operator<<` overloads. Refs CIVET failure https://civet.inl.gov/job/2295949/ on libMesh/libmesh#3883

I added this at a time when I thought I would add a test with a field split of a statically condensed system, but I never did. So the need is no longer there and Roy hates it so why not remove

lindsayad · 2024-09-24T22:37:06Z

MOOSE failure is unrelated and already codified in idaholab/moose#27555

lindsayad · 2024-09-24T23:35:06Z

This should be ready for another round of review

lindsayad force-pushed the static-condensation branch 3 times, most recently from bdb4d4e to 21e8140 Compare June 24, 2024 15:33

lindsayad changed the title ~~[WIP] Static condensation~~ Static condensation Jun 24, 2024

lindsayad force-pushed the static-condensation branch 5 times, most recently from aa8246a to 1468055 Compare June 25, 2024 18:47

lindsayad added a commit to lindsayad/moose that referenced this pull request Jun 26, 2024

Make ConsoleStream thread-safe

4246007

Use the same mutex for both `operator<<` overloads. Refs CIVET failure https://civet.inl.gov/job/2295949/ on libMesh/libmesh#3883

lindsayad mentioned this pull request Jun 26, 2024

Make ConsoleStream/GeneralRegistry thread-safe idaholab/moose#27996

Merged

lindsayad force-pushed the static-condensation branch from 3a98105 to 11f7769 Compare June 26, 2024 14:15

lindsayad marked this pull request as ready for review June 26, 2024 17:54

lindsayad requested a review from roystgnr June 26, 2024 17:54

lindsayad self-assigned this Jun 26, 2024

lindsayad mentioned this pull request Jun 26, 2024

Implement static condensation #3891

Open

5 tasks

roystgnr reviewed Jun 27, 2024

View reviewed changes

examples/vector_fe/vector_fe_ex9/run.sh Show resolved Hide resolved

roystgnr requested changes Jun 27, 2024

View reviewed changes

lindsayad marked this pull request as draft July 1, 2024 19:36

This was referenced Jul 1, 2024

Add LIBMESH_PETSC_SUCCESS #3894

Merged

Add fuzzy equals comparisons #3895

Merged

A couple small changes for subvectors #3896

Merged

Create Elem::is_internal API for local node indices #3897

Merged

lindsayad changed the title ~~Static condensation~~ [WIP] Static condensation Jul 5, 2024

lindsayad force-pushed the static-condensation branch from 9491d1f to 6818bf3 Compare July 30, 2024 19:38

lindsayad changed the title ~~[WIP] Static condensation~~ Static condensation Jul 30, 2024

lindsayad force-pushed the static-condensation branch 4 times, most recently from f4f4afe to da85e83 Compare July 31, 2024 00:06

lindsayad force-pushed the static-condensation branch 2 times, most recently from 32b4e29 to e60b60c Compare August 14, 2024 15:11

lindsayad marked this pull request as ready for review August 14, 2024 20:49

roystgnr reviewed Aug 26, 2024

View reviewed changes

roystgnr requested changes Aug 26, 2024

View reviewed changes

Leni-Yeo pushed a commit to Leni-Yeo/moose that referenced this pull request Aug 28, 2024

Make ConsoleStream thread-safe

92af9e8

Use the same mutex for both `operator<<` overloads. Refs CIVET failure https://civet.inl.gov/job/2295949/ on libMesh/libmesh#3883

lindsayad added 11 commits September 24, 2024 13:07

Create a static condensation class

474d00c

Don't use const value for params in function declarations

15396fd

Clean up somehow mucked up indentation

2dbddc1

Remove unused or singly-used dof methods

0507708

Add .clang-format file

a8c3c7d

Apply clang-formatting to StaticCondensation

b10893d

Add lots of doxygen

a8b30bc

Remove _system from LinearSolver

6167f7a

I added this at a time when I thought I would add a test with a field split of a statically condensed system, but I never did. So the need is no longer there and Roy hates it so why not remove

KSP snes -> KSP ksp

6dbfae8

PetscMatrixBase -> PetscMatrix so comments match code

9b3cf83

Allow requesting static condensation from code

31388ef

lindsayad force-pushed the static-condensation branch from 0b22214 to 31388ef Compare September 24, 2024 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Static condensation #3883

Static condensation #3883

lindsayad commented Jun 20, 2024 •

edited

Loading

moosebuild commented Jun 26, 2024

moosebuild commented Jun 26, 2024 •

edited

Loading

lindsayad commented Jun 26, 2024

roystgnr left a comment

roystgnr Jun 27, 2024

lindsayad commented Jul 1, 2024

lindsayad commented Jul 1, 2024

lindsayad commented Jul 5, 2024

moosebuild commented Aug 14, 2024

moosebuild commented Aug 14, 2024

moosebuild commented Aug 14, 2024

lindsayad commented Aug 14, 2024

roystgnr Aug 26, 2024

jwpeterson Aug 26, 2024

roystgnr Aug 26, 2024

lindsayad Sep 19, 2024 •

edited

Loading

roystgnr Sep 24, 2024

roystgnr Aug 26, 2024

roystgnr Aug 26, 2024

roystgnr Aug 26, 2024

lindsayad Sep 19, 2024

roystgnr Sep 24, 2024

roystgnr Aug 26, 2024

lindsayad Sep 19, 2024

roystgnr Sep 24, 2024

roystgnr Aug 26, 2024

roystgnr Aug 26, 2024

roystgnr Aug 26, 2024

roystgnr Aug 26, 2024

lindsayad commented Sep 24, 2024

lindsayad commented Sep 24, 2024

		{
		if (libMesh::on_command_line("--" + name_in + "-static-condensation"))

Static condensation #3883

Are you sure you want to change the base?

Static condensation #3883

Conversation

lindsayad commented Jun 20, 2024 • edited Loading

moosebuild commented Jun 26, 2024

moosebuild commented Jun 26, 2024 • edited Loading

Coverage

Warnings

lindsayad commented Jun 26, 2024

roystgnr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindsayad commented Jul 1, 2024

lindsayad commented Jul 1, 2024

lindsayad commented Jul 5, 2024

moosebuild commented Aug 14, 2024

moosebuild commented Aug 14, 2024

moosebuild commented Aug 14, 2024

lindsayad commented Aug 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindsayad Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindsayad commented Sep 24, 2024

lindsayad commented Sep 24, 2024

lindsayad commented Jun 20, 2024 •

edited

Loading

moosebuild commented Jun 26, 2024 •

edited

Loading

lindsayad Sep 19, 2024 •

edited

Loading