fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells #3834

paveltomin · 2025-09-25T17:27:17Z

~~this is preparatory cleanup for fixing #3823~~
in setupSystem, matrix pattern build is moved into a separate function to allow code re-use and calling pattern build separately from full setup
merged changes from #3839

Copilot

Pull Request Overview

This PR refactors the matrix sparsity pattern setup logic by extracting the pattern creation into a separate virtual method. The change separates concerns between system setup and sparsity pattern creation, allowing derived classes to override only the pattern logic.

Key changes:

Introduces a new virtual setSparsityPattern() method across solver classes
Removes redundant destructor implementations
Updates parameter ordering for consistency (domain before dofManager)

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
PhysicsSolverBase.hpp/.cpp	Adds base `setSparsityPattern()` method with default implementation
SolidMechanicsLagrangianFEM.hpp/.cpp	Extracts sparsity pattern logic and updates `setupSystem()` signature
SolidMechanicsPenaltyContact.hpp/.cpp	Replaces `setupSystem()` with `setSparsityPattern()`, removes destructor
SolidMechanicsEmbeddedFractures.hpp/.cpp	Refactors to use new pattern method, removes destructor
LaplaceFEM.hpp/.cpp	Converts to `setSparsityPattern()` method, removes destructor
Contact solver classes	Updates to new pattern separation approach
Multiphysics solver classes	Adopts new sparsity pattern method

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-25T20:38:25Z

src/coreComponents/physicsSolvers/solidMechanics/contact/SolidMechanicsEmbeddedFractures.cpp

+  if( m_useStaticCondensation )
  {
+    SolidMechanicsLagrangianFEM::setSparsityPattern( domain, dofManager, pattern );
+    return;
+  }


The logic is inverted. The original code had if( !m_useStaticCondensation ) and executed the custom sparsity pattern logic. Now it's if( m_useStaticCondensation ) but calls the base class method, which reverses the intended behavior.

Copilot · 2025-09-25T20:38:26Z

...omponents/physicsSolvers/solidMechanics/contact/SolidMechanicsAugmentedLagrangianContact.cpp


  // Add the number of nonzeros induced by coupling
-  this->addCouplingNumNonzeros( domain, dofManager, rowLengths.toView() );
+  addCouplingNumNonzeros( domain, dofManager, rowLengths.toView());


[nitpick] Inconsistent spacing in method calls. Line 234 has no space before the opening parenthesis, while line 247 has a space. The spacing should be consistent throughout the method.

Copilot · 2025-09-25T20:38:26Z

...omponents/physicsSolvers/solidMechanics/contact/SolidMechanicsAugmentedLagrangianContact.cpp

-  solution.setName( this->getName() + "/solution" );
-  solution.create( dofManager.numLocalDofs(), MPI_COMM_GEOS );
-
+  addCouplingSparsityPattern( domain, dofManager, pattern.toView());


[nitpick] Inconsistent spacing in method calls. Line 234 has no space before the opening parenthesis, while line 247 has a space. The spacing should be consistent throughout the method.

Suggested change

addCouplingSparsityPattern( domain, dofManager, pattern.toView());

addCouplingSparsityPattern(domain, dofManager, pattern.toView());

…o pt/set-pattern

Removed USE_SCCACHE environment variable from CI workflow.

paveltomin · 2025-09-26T18:03:48Z

this is ready for review

castelletto1 · 2025-09-29T23:24:50Z

src/coreComponents/physicsSolvers/PhysicsSolverBase.cpp

+void PhysicsSolverBase::setSparsityPattern( DomainPartition & GEOS_UNUSED_PARAM( domain ),
+                                            DofManager & dofManager,
+                                            SparsityPattern< globalIndex > & pattern )
+{
+  dofManager.setSparsityPattern( pattern );


At some point we may want to adopt a consistent style and chose between GEOS_UNUSED_PARAM and GEOS_UNUSED_VAR.

Added parameter documentation for localMatrix in setSparsityPattern.

refactor: clean up set sparsity pattern logic

8209ece

paveltomin self-assigned this Sep 25, 2025

paveltomin added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Sep 25, 2025

paveltomin requested review from CusiniM, Guotong-Ren, OmarDuran, castelletto1, frankfeifan, jhuang2601, matteofrigo5 and rrsettgast as code owners September 25, 2025 17:27

Pavel Tomin and others added 4 commits September 25, 2025 12:32

code style and doxygen

2cb7e06

missing override

61dab0f

fix

6840707

Merge branch 'develop' into pt/set-pattern

9c242ec

paveltomin requested a review from Copilot September 25, 2025 20:37

Copilot AI reviewed Sep 25, 2025

View reviewed changes

paveltomin and others added 4 commits September 25, 2025 16:47

Merge branch 'develop' into pt/set-pattern

87ee761

bug fix

19e1d17

Merge branch 'pt/set-pattern' of https://github.com/GEOS-DEV/GEOS int…

52370d3

…o pt/set-pattern

Fix formatting in Group.hpp

000e415

paveltomin requested review from MelReyCG, corbett5 and wrtobin as code owners September 26, 2025 14:50

paveltomin added 3 commits September 26, 2025 10:29

Disable SCCACHE in CI workflow

5392697

Remove USE_SCCACHE from CI tests configuration

8619b94

Removed USE_SCCACHE environment variable from CI workflow.

Remove unnecessary blank line in Group.hpp

810defa

paveltomin added type: cleanup / refactor Non-functional change (NFC) flag: ready for review flag: no rebaseline Does not require rebaseline labels Sep 26, 2025

Merge branch 'develop' into pt/set-pattern

ec946b0

paveltomin removed request for MelReyCG, corbett5 and wrtobin September 26, 2025 18:04

paveltomin mentioned this pull request Sep 26, 2025

fix: matrix setup for poromechanics with contact and wells #3839

Merged

castelletto1 reviewed Sep 29, 2025

View reviewed changes

castelletto1 approved these changes Sep 29, 2025

View reviewed changes

fix: matrix setup for poromechanics with contact and wells (#3839)

137c6e9

paveltomin changed the title ~~refactor: clean up set sparsity pattern logic~~ fix: clean up set sparsity pattern logic Sep 29, 2025

paveltomin changed the title ~~fix: clean up set sparsity pattern logic~~ fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells Sep 29, 2025

paveltomin added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Sep 29, 2025

paveltomin added 3 commits September 29, 2025 18:40

Merge branch 'develop' into pt/set-pattern

2001883

Document localMatrix parameter in setSparsityPattern

1ae2748

Added parameter documentation for localMatrix in setSparsityPattern.

Merge branch 'develop' into pt/set-pattern

ee1e2ac

paveltomin merged commit 85f0714 into develop Sep 30, 2025
25 of 26 checks passed

paveltomin deleted the pt/set-pattern branch September 30, 2025 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells #3834

fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells #3834

Uh oh!

paveltomin commented Sep 25, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 25, 2025

Uh oh!

Copilot AI Sep 25, 2025

Uh oh!

Copilot AI Sep 25, 2025

Uh oh!

paveltomin commented Sep 26, 2025

Uh oh!

castelletto1 Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	addCouplingSparsityPattern( domain, dofManager, pattern.toView());
	addCouplingSparsityPattern(domain, dofManager, pattern.toView());

fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells #3834

fix: clean up set sparsity pattern logic + matrix setup for poromechanics with contact and wells #3834

Uh oh!

Conversation

paveltomin commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

paveltomin commented Sep 26, 2025

Uh oh!

castelletto1 Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paveltomin commented Sep 25, 2025 •

edited

Loading