Refactor projection #103

cbritopacheco · 2025-08-19T21:10:35Z

No description provided.

Copilot

Pull Request Overview

This pull request refactors projection functionality across the Rodin finite element codebase. The changes replace separate projection methods (projectOnCells, projectOnBoundary, etc.) with a unified project method that takes a Region parameter, simplifying the API and improving consistency.

Unified projection API using Region parameter instead of separate methods for different regions
Replaced region-specific projection methods with a single project method
Updated method calls throughout tests and benchmarks to use the new projection API

Reviewed Changes

Copilot reviewed 131 out of 131 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/unit/Rodin/Variational/P1Test.cpp	Updated test calls to use new unified projection API with Region parameters
tests/benchmarks/P1.cpp	Updated benchmark calls to use new unified projection API with Region parameters
src/Rodin/Variational/GridFunction.h	Refactored projection methods into unified project method with Region parameter
src/Rodin/Variational/*.h	Various header files with simplified function signatures and removed unused methods
src/Rodin/Variational/*.cpp	Removed many implementation files that were empty or contained only namespace declarations

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

Copilot · 2025-08-19T21:12:37Z

src/Rodin/Variational/Zero.h

-
-      constexpr
-      auto getValue(const Geometry::Point&) const
+      decltype(auto) getValue(const Geometry::Point&) const


The return type decltype(auto) for a constant value (0) is unnecessarily complex. Consider returning constexpr ScalarType instead for better clarity and performance.

Suggested change

decltype(auto) getValue(const Geometry::Point&) const

ScalarType getValue(const Geometry::Point&) const

Copilot · 2025-08-19T21:12:38Z

src/Rodin/Variational/VectorFunction.h

+          {
+            s_res.coeffRef(static_cast<Eigen::Index>(i)) = std::get<i>(m_fs).getValue(p);
+          });
+        return s_res;


Using thread_local static variables in template functions can lead to memory bloat as each template instantiation creates its own thread-local storage. Consider using a function-local static variable or passing the result vector as a parameter.

Copilot · 2025-08-19T21:12:38Z

src/Rodin/Variational/P1/P1Element.h

+          ScalarType operator()(const T& v) const
          {
-            return v(P1Element<ScalarType>(m_g).getNode(m_local / m_vdim)).coeff(m_local % m_vdim);
+            static thread_local RangeType s_out;


Multiple thread_local static variables in template functions can cause excessive memory usage. Each template instantiation creates separate thread-local storage, which may not be necessary for temporary calculations.

Suggested change

static thread_local RangeType s_out;

RangeType s_out;

Copilot · 2025-08-19T21:12:38Z

src/Rodin/Variational/Potential.h

        const auto& kernel = getKernel();
        const auto& operand = getOperand();
        const auto& mesh = p.getPolytope().getMesh();
        assert(false);


The assert(false) statement will always trigger in debug builds, making this code path unreachable. This appears to be incomplete code that should either be implemented or removed.

Suggested change

assert(false);

Copilot · 2025-08-19T21:12:38Z

src/Rodin/Variational/GridFunction.h

+          {
+            it = mesh.getInterface();
+            break;
+          }


The switch statement for region handling lacks a default case. Consider adding a default case with an assertion or error handling for unexpected region values.

Suggested change

}

}

default:

{

assert(false && "Unexpected region value in switch statement");

break;

}

Copilot · 2025-08-19T21:12:39Z

src/Rodin/Variational/Mult.h

  template <class LHSDerived>
  auto
-  operator*(const FunctionBase<LHSDerived>& lhs, std::reference_wrapper<const Math::Matrix<Real>> rhs)
+  operator*(const FunctionBase<LHSDerived>& lhs, const Math::Matrix<Real>& rhs)


[nitpick] Inconsistent parameter passing: this function takes const Math::Matrix<Real>& while similar functions use different reference types. Consider standardizing reference passing for matrix operations.

codecov · 2025-08-19T21:54:21Z

Codecov Report

❌ Patch coverage is 82.06522% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.95%. Comparing base (72cb224) to head (e2feab1).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/Rodin/Variational/GridFunction.h	78.37%	8 Missing ⚠️
src/Rodin/Assembly/OpenMP.cpp	50.00%	5 Missing ⚠️
src/Rodin/Variational/F.h	77.27%	5 Missing ⚠️
src/Rodin/Assembly/Sequential.cpp	40.00%	3 Missing ⚠️
src/Rodin/Variational/BoundaryIntegral.h	0.00%	3 Missing ⚠️
src/Rodin/Variational/InterfaceIntegral.h	0.00%	3 Missing ⚠️
src/Rodin/Variational/QuadratureRule.h	25.00%	3 Missing ⚠️
src/Rodin/Variational/FaceIntegral.h	0.00%	2 Missing ⚠️
src/Rodin/Variational/RealFunction.h	85.71%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #103      +/-   ##
===========================================
- Coverage    65.36%   64.95%   -0.42%     
===========================================
  Files          129      129              
  Lines         6852     6797      -55     
===========================================
- Hits          4479     4415      -64     
- Misses        2373     2382       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

* Update * Update. * CI * CI * Update * Update * Refactor * Switch to OpenMP for multithreading * Remove thread pool * CI * Refactor * CI * CI * Workon MPI Poisson example * Will refactor GridFunction * Refactor * Update * Update * Small include error * Ownership and ghost * Fix * Fix * Update * GridFunction refactor (#102) PETSc * CI * CI * CI * CI * CI * CI * Refactor projection (#103) * Update * Refactor project and getValue * CI * CI * Add Release test build into matrix * Remove boost::bimap and fix tests * CI * CI * CI * CI * CI * cI * CI * Add comprehensive unit tests for Rodin Variational mathematical operations (#105) * Initial plan * Add comprehensive unit tests for core Rodin Variational classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive unit tests for additional core Rodin Variational classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Complete comprehensive unit test suite for Rodin Variational classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Remove any mention of RangeShape * Remove RangeShape references from DotTest, DivTest, and ComponentTest Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Remove all remaining RangeShape references from Rodin Variational test files Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix Copilot code * Add comprehensive unit tests for fundamental mathematical operations in Rodin Variational Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive unit tests for advanced mathematical operations in Rodin Variational Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add tests * CI * CI * CI * CI --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> Co-authored-by: Carlos Brito <carlos.brito524@gmail.com> * Add comprehensive Doxygen documentation throughout the codebase (#111) * Initial plan * Add Doxygen documentation to Math, QF, IO, Solver modules and core types Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add documentation to QF, Models, MPI, and Test modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add documentation to core utility classes - Moveable and GeometryIndexed Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive documentation to FormLanguage, Assembly, Variational, Geometry, and Context modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add documentation to additional Variational, Geometry, and Assembly components Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Complete documentation coverage with FiniteElementSpace and Plot modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix Copyable documentation and add comprehensive docs to Alert and Utility modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix build --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> Co-authored-by: Carlos Brito <carlos.brito524@gmail.com> * Add Helmholtz manufactured test * Add comprehensive unit tests for all major Rodin modules and classes (#112) * Initial plan * Initial assessment: Identify missing unit tests for src/Rodin classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add unit tests for Copyable, Moveable, FlatSet, Types, and Cast classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix * Add unit tests for Utility, Alert, and QF classes Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Put the QF tests inside their own folder * Split QF tests into separate files by class Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive unit tests for Math, Test, and FormLanguage modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive unit tests for Plot, Models, Solver, and Assembly modules Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix tests * Split Alert tests into separate files by class in Alert directory Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Add comprehensive unit tests for Math module classes Unit, Rad, Traits, and LinearSystem Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Update README with logo * Fix tests * Update logo * Revert submodules to match develop branch state Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Remove QF abstract base class from tests * Fix GaussLegendre * Add comprehensive unit tests for GaussLegendre class Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> * Fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com> Co-authored-by: Carlos Brito <carlos.brito524@gmail.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: cbritopacheco <6352283+cbritopacheco@users.noreply.github.com>

cbritopacheco added 2 commits August 8, 2025 16:22

Update

75be8bf

Refactor project and getValue

5ebe73b

github-actions bot added geometry Rodin::Geometry variational Rodin::Variational mmg Rodin::External::MMG examples Examples benchmarks Benchmarks unit-tests Unit tests math Rodin::Math io Rodin::IO labels Aug 19, 2025

cbritopacheco requested a review from Copilot August 19, 2025 21:11

Copilot AI reviewed Aug 19, 2025

View reviewed changes

cbritopacheco added 2 commits August 19, 2025 23:24

CI

5bf6197

CI

e2feab1

cbritopacheco merged commit 2f17d8b into develop Aug 19, 2025
9 of 18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor projection #103

Refactor projection #103

Uh oh!

cbritopacheco commented Aug 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

Copilot AI Aug 19, 2025

Uh oh!

codecov bot commented Aug 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	decltype(auto) getValue(const Geometry::Point&) const
	ScalarType getValue(const Geometry::Point&) const

-          }
+          }
+          default:
+          {
+            assert(false && "Unexpected region value in switch statement");
+            break;
+          }

Uh oh!

Refactor projection #103

Refactor projection #103

Uh oh!

Conversation

cbritopacheco commented Aug 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Aug 19, 2025 •

edited

Loading