fix: enable FullyImplicit for SinglePhaseReservoirPoromechanicsConformingFractures#3851
Merged
paveltomin merged 13 commits intodevelopfrom Oct 13, 2025
Merged
fix: enable FullyImplicit for SinglePhaseReservoirPoromechanicsConformingFractures#3851paveltomin merged 13 commits intodevelopfrom
paveltomin merged 13 commits intodevelopfrom
Conversation
paveltomin
requested review from
CusiniM,
MelReyCG,
corbett5,
rrsettgast and
wrtobin
as code owners
paveltomin
added
the
ci: run integrated tests
paveltomin
requested review from
Guotong-Ren,
OmarDuran,
castelletto1,
dkachuma,
frankfeifan and
jhuang2601
as code owners
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enables the FullyImplicit feature for SinglePhaseReservoirPoromechanicsConformingFractures by refactoring the assembly system architecture. The changes move common assembly logic to the base PoromechanicsSolver class and update derived classes to leverage this shared implementation.
Key changes:
- Moves assembleSystem implementation from derived classes to PoromechanicsSolver base class
- Adds virtual assembleElementBasedTerms method for derived class customization
- Updates error messages to use consistent single quote formatting
- Adds getDiscretizationName override for coupled reservoir and wells
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SinglePhasePoromechanicsConformingFractures.hpp | Adds assembleSystem override and removes private method declaration |
| SinglePhasePoromechanicsConformingFractures.cpp | Implements template specialization for SinglePhaseReservoirAndWells assembly |
| SinglePhasePoromechanics.hpp | Updates assembleSystem to use base class implementation |
| SinglePhasePoromechanics.cpp | Removes duplicate assembly logic, delegates to base class |
| PoromechanicsSolver.hpp | Adds common assembleSystem implementation and virtual assembleElementBasedTerms |
| MultiphasePoromechanics.hpp | Updates to use base class assembleSystem implementation |
| MultiphasePoromechanics.cpp | Removes duplicate assembly logic, delegates to base class |
| CoupledReservoirAndWellsBase.hpp | Adds virtual getDiscretizationName override |
| PhysicsSolverBase.hpp | Makes getDiscretizationName virtual |
| MeshObjectPath.cpp | Updates error message to use single quotes |
| testGroupPath.cpp | Updates test expectation for new error message format |
| Group.hpp | Updates error messages to use single quotes for consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/coreComponents/physicsSolvers/multiphysics/SinglePhasePoromechanicsConformingFractures.hpp
Show resolved
Hide resolved
src/coreComponents/physicsSolvers/multiphysics/SinglePhasePoromechanics.cpp
Show resolved
Hide resolved
paveltomin
added
type: bug
Collaborator
Author
|
Ready for review |
dkachuma
approved these changes
Oct 8, 2025
Contributor
dkachuma
left a comment
dkachuma
left a comment
There was a problem hiding this comment.
Looks good to me. This is also done for multiphase, right?
Collaborator
Author
Once wells are enabled for multiphase, similar version of assembleSystem function needs to be added there, not done yet in this PR |
paveltomin
commented
Oct 9, 2025
| Base::postInputInitialization(); | ||
|
|
||
| // assume that reservoir solver discretization is the primary one | ||
| this->m_discretizationName = reservoirSolver()->getDiscretizationName(); |
Collaborator
Author
There was a problem hiding this comment.
this is first fix
paveltomin
commented
Oct 9, 2025
|
|
||
| Base::assembleSystem( time_n, dt, domain, dofManager, localMatrix, localRhs ); | ||
|
|
||
| // assemble well contributions |
Collaborator
Author
There was a problem hiding this comment.
this is second fix
paveltomin
added
flag: requires rebaseline
jhuang2601
approved these changes
Oct 10, 2025
Collaborator
Author
|
@castelletto1 @OmarDuran @frankfeifan @Guotong-Ren review please |
castelletto1
approved these changes
Oct 13, 2025
Collaborator
castelletto1
left a comment
castelletto1
left a comment
There was a problem hiding this comment.
Looks good to me, thanks.
paveltomin
added a commit
that referenced
this pull request
Oct 28, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
should fix #3850
fixes are:
the rest of the changes is cleanup - move code to parent class where possible to avoid duplication