Skip to content

feat: inner products for mixed method - #3994

Merged
victorapm merged 27 commits into
developfrom
feature/inner_products_for_mixed_method
Sep 4, 2026
Merged

feat: inner products for mixed method#3994
victorapm merged 27 commits into
developfrom
feature/inner_products_for_mixed_method

Conversation

@chauj96

@chauj96 chauj96 commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

This PR adds the implementation of computeM, which computes the inner product matrix $M$ used in the MFD mixed method formulation.

  1. The function has been implemented for the following inner product types:
  • TPFAInnerProduct
  • BdVLMInnerProduct
  • QuasiTPFAInnerProduct
  • SimpleInnerProduct
    Each class now includes a computeM routine that construct the inner product matrix $M$ required for the mixed formulation.
  1. Testing
    Unit tests were added in testMimeticInnerProducts to verify the correctness of the newly implemented computeM functions. All tests currently pass.

  2. The centroid computation has been updated to evaluate the exact volume of a polyhedron with planar polygonal facets. This property is verified by a unit test. This induces a rebaseline.

+) Minor variable naming updates were applied in SimpleInnerProduct to keep the geometry nomenclature consistent with the other inner product implemenations.

@chauj96 chauj96 changed the title Feature/inner products for mixed method feat:inner products for mixed method Mar 12, 2026
@chauj96 chauj96 self-assigned this Mar 12, 2026
@chauj96 chauj96 added ci: run CUDA builds ci: run integrated tests Allows to run the integrated tests in GEOS CI ci: run code coverage enables running of the code coverage CI jobs flag: no rebaseline Does not require rebaseline labels Mar 12, 2026
Comment thread src/coreComponents/finiteVolume/mimeticInnerProducts/TPFAInnerProduct.hpp Outdated
Comment thread src/coreComponents/finiteVolume/mimeticInnerProducts/SimpleInnerProduct.hpp Outdated
Comment thread src/coreComponents/finiteVolume/mimeticInnerProducts/SimpleInnerProduct.hpp Outdated
@chauj96 chauj96 changed the title feat:inner products for mixed method feat: inner products for mixed method Mar 16, 2026
@chauj96
chauj96 marked this pull request as ready for review March 16, 2026 23:36
@OmarDuran
OmarDuran requested review from bd713 and jafranc as code owners July 31, 2026 19:56

@OmarDuran OmarDuran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @chauj96, nice work.

@OmarDuran
OmarDuran requested a review from wrtobin as a code owner August 4, 2026 01:36

@bd713 bd713 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update the PR description to clearly explain that the change in centroid computation is the reason a rebaseline is needed.

@OmarDuran OmarDuran added flag: requires rebaseline Requires rebaseline branch in integratedTests and removed flag: no rebaseline Does not require rebaseline labels Aug 4, 2026

@jafranc jafranc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on rewriting as inner products rather than transmissivities.

Although I am unclear on why it is not called in compute replacing the old way and just duplicated from it up to the transposition.

Comment thread src/coreComponents/mesh/utilities/ComputationalGeometry.hpp
@OmarDuran OmarDuran self-assigned this Sep 2, 2026
@OmarDuran OmarDuran added the ci: run device builds Allows to run the device (CUDA/HIP) builds label Sep 2, 2026
@OmarDuran

Copy link
Copy Markdown
Contributor

Great work on rewriting as inner products rather than transmissivities.

Although I am unclear on why it is not called in compute replacing the old way and just duplicated from it up to the transposition.

Thanks! The reason computeM doesn't replace the body of compute is that they build different matrices: compute assembles the inverse inner product W = (N K Nᵀ + t·P_C diag(N K Nᵀ) P_C)/V for the hybrid (flux-elimination) scheme, while computeM assembles the primal M = C K⁻¹ Cᵀ/V + (V/t)·P_N diag(N K Nᵀ)⁻¹ P_N for the mixed saddle-point formulation, where fluxes stay unknowns. The dual structure of M↔W (C↔N, K↔K⁻¹, t↔1/t) exists by consistency construction, but the stabilizations differ, so in general M ≠ W⁻¹, if we want to call one from the other would need a dense per-element inversion in-kernel. The actual shared part is now factored into computeCellToFaceGeometry (C, N, face areas) and computeConsistencyTerm (C K⁻¹ Cᵀ/V).

@jafranc

jafranc commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Great refactor 👍

@victorapm
victorapm merged commit fb09670 into develop Sep 4, 2026
22 checks passed
@victorapm
victorapm deleted the feature/inner_products_for_mixed_method branch September 4, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run code coverage enables running of the code coverage CI jobs ci: run device builds Allows to run the device (CUDA/HIP) builds ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: requires rebaseline Requires rebaseline branch in integratedTests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants