Enh/k omega sst - #554
Draft
greole wants to merge 61 commits into
Draft
Conversation
|
Thank you for your PR, here are some useful tips:
|
greole
force-pushed
the
enh/kOmegaSST
branch
2 times, most recently
from
June 21, 2026 13:23
cb95d4c to
4778a2c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR adds support for implicit transform boundary conditions (slip/symmetry) via per-component diagonal corrections, extends boundary-condition infrastructure (including flux-aware inletOutlet), and introduces a bounded divergence operator to improve positivity in convection discretizations.
Changes:
- Add
diagCmptstorage toLinearSystemand update Gauss-Green Laplacian assembly + Ginkgo solvers (serial + distributed) to apply per-component diagonal corrections for implicit slip/symmetry. - Add new volume BCs (
slip,inletOutlet) and extendBoundaryContextto carry surface-scalar fields (e.g., face fluxphi). - Introduce
boundedDivoperator wrapper and add/extend unit tests for BC behavior and implicit-transform solver paths.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/linearAlgebra/ginkgo.cpp | Adds unit test covering implicit transform diag correction in serial Ginkgo solve. |
| test/finiteVolume/cellCentred/operator/laplacianOperator.cpp | Adds unit test verifying Laplacian assembly populates diagCmpt for implicit slip. |
| test/finiteVolume/cellCentred/boundary/volume/volSymmetry.cpp | Extends symmetry tests to cover deferred vs implicit modes. |
| test/finiteVolume/cellCentred/boundary/volume/volSlip.cpp | New tests for slip BC (scalar + vector, deferred + implicit). |
| test/finiteVolume/cellCentred/boundary/volume/volInletOutlet.cpp | New tests for inletOutlet BC with/without phi context (scalar + vector). |
| test/finiteVolume/cellCentred/boundary/volume/CMakeLists.txt | Registers new unit tests. |
| test/distributed/operator.cpp | Adds distributed unit test for implicit transform diagonal correction solve. |
| src/linearAlgebra/ginkgo/ginkgoDistributed.cpp | Implements distributed implicit-transform component solve by in-place diagonal edits. |
| src/linearAlgebra/ginkgo/ginkgo.cpp | Implements serial implicit-transform component solve by in-place diagonal edits. |
| src/finiteVolume/cellCentred/operators/gaussGreenLaplacian.cpp | Accumulates implicit transform damping into diagCmpt instead of shared diagonal. |
| src/finiteVolume/cellCentred/operators/boundedDiv.cpp | New bounded divergence operator implementation. |
| src/finiteVolume/cellCentred/boundary/boundaryContext.cpp | Adds support for inserting/reading surface-scalar fields in BoundaryContext. |
| src/CMakeLists.txt | Adds boundedDiv.cpp to build. |
| include/NeoN/linearAlgebra/linearSystem.hpp | Adds diagCmpt storage + deep copy/reset/copyToExecutor support. |
| include/NeoN/finiteVolume/cellCentred/operators/boundedDiv.hpp | Declares bounded divergence operator wrapper. |
| include/NeoN/finiteVolume/cellCentred/boundary/volumeBoundaryFactory.hpp | Adds transformImplicit attribute flag for BCs. |
| include/NeoN/finiteVolume/cellCentred/boundary/volume/symmetry.hpp | Refactors symmetry to share slip/symmetry implementation and expose implicit mode flag. |
| include/NeoN/finiteVolume/cellCentred/boundary/volume/slip.hpp | Introduces slip BC (deferred/implicit normal damping). |
| include/NeoN/finiteVolume/cellCentred/boundary/volume/inletOutlet.hpp | Introduces flux-dependent inletOutlet BC using BoundaryContext surface flux. |
| include/NeoN/finiteVolume/cellCentred/boundary/volume/detail/slipSymmetry.hpp | Shared slip/symmetry implementation + tolerant parsing of "implicit". |
| include/NeoN/finiteVolume/cellCentred/boundary/boundaryContext.hpp | Adds surface-scalar field slots to BoundaryContext. |
| include/NeoN/finiteVolume/cellCentred/boundary.hpp | Registers slip and inletOutlet boundary types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+545
to
+553
| parallelFor( | ||
| exec, | ||
| {0, nrows}, | ||
| NEON_LAMBDA(const localIdx cell) { | ||
| Kokkos::atomic_sub(&values[ma.diagIdx(cell)], diagC[cell][I]); | ||
| }, | ||
| "applyImplicitTransformDiag" | ||
| ); | ||
| gkoExec->synchronize(); |
Comment on lines
+454
to
+462
| parallelFor( | ||
| exec, | ||
| {0, nrows}, | ||
| NEON_LAMBDA(const localIdx cell) { | ||
| Kokkos::atomic_sub(&values[ma.diagIdx(cell)], diagC[cell][I]); | ||
| }, | ||
| "applyImplicitTransformDiagDist" | ||
| ); | ||
| gkoExec->synchronize(); |
Comment on lines
+548
to
+550
| NEON_LAMBDA(const localIdx cell) { | ||
| Kokkos::atomic_sub(&values[ma.diagIdx(cell)], diagC[cell][I]); | ||
| }, |
Comment on lines
+198
to
+201
| [[nodiscard]] const std::shared_ptr<Vector<RHSValueType>>& diagCmpt() const | ||
| { | ||
| return diagCmpt_; | ||
| } |
Comment on lines
44
to
+47
| const VolumeField<scalar>& scalarFieldPtr(const std::string& name) const; | ||
| const VolumeField<Vec3>& vectorFieldPtr(const std::string& name) const; | ||
| const VolumeField<Tensor>& tensorFieldPtr(const std::string& name) const; | ||
| const SurfaceField<scalar>& surfaceScalarField(const std::string& name) const; |
Comment on lines
+270
to
+287
| void BoundedDiv<FieldValueType, AssemblyType>::div( | ||
| la::LinearSystem<AssemblyType, FieldValueType>& ls, | ||
| const SurfaceField<scalar>& faceFlux, | ||
| const VolumeField<FieldValueType>& phi, | ||
| const dsl::Coeff operatorScaling | ||
| ) const | ||
| { | ||
| inner_->div(ls, faceFlux, phi, operatorScaling); | ||
| applyBoundedDiagInternal<FieldValueType, AssemblyType>( | ||
| ls, faceFlux, this->mesh_, operatorScaling | ||
| ); | ||
| applyBoundedDiagBoundary<FieldValueType, AssemblyType>( | ||
| ls, faceFlux, this->mesh_, operatorScaling | ||
| ); | ||
| applyBoundedDiagProcBoundary<FieldValueType, AssemblyType>( | ||
| ls, faceFlux, this->mesh_, operatorScaling | ||
| ); | ||
| } |
Comment on lines
+596
to
+605
| SolverStats stats; | ||
| solveImplicitTransformComponent<0>( | ||
| sys, x, exec_, gkoExec_, gkoMtx, factory_, stats, l1Control, values, ma, diagC, nrows | ||
| ); | ||
| solveImplicitTransformComponent<1>( | ||
| sys, x, exec_, gkoExec_, gkoMtx, factory_, stats, l1Control, values, ma, diagC, nrows | ||
| ); | ||
| solveImplicitTransformComponent<2>( | ||
| sys, x, exec_, gkoExec_, gkoMtx, factory_, stats, l1Control, values, ma, diagC, nrows | ||
| ); |
greole
commented
Jun 21, 2026
Comment on lines
+525
to
+580
| if (sys.diagCmpt() && sys.diagCmpt()->size() > 0) | ||
| { | ||
| auto values = const_cast<Vector<scalar>&>(sys.matrix().values()).view(); | ||
| const auto ma = sys.faceToMatrixAddress()->view(sys.matrix().rowOffs().view()); | ||
| auto diagC = sys.diagCmpt()->view(); | ||
| const localIdx nrows = sys.rhs().size(); | ||
| gkoExec_->synchronize(); | ||
|
|
||
| SolverStats stats; | ||
| solveImplicitTransformComponentDist<0>( | ||
| sys, | ||
| x, | ||
| exec_, | ||
| gkoExec_, | ||
| comm, | ||
| gkoMtx, | ||
| factory_, | ||
| stats, | ||
| l1Control, | ||
| values, | ||
| ma, | ||
| diagC, | ||
| nrows | ||
| ); | ||
| solveImplicitTransformComponentDist<1>( | ||
| sys, | ||
| x, | ||
| exec_, | ||
| gkoExec_, | ||
| comm, | ||
| gkoMtx, | ||
| factory_, | ||
| stats, | ||
| l1Control, | ||
| values, | ||
| ma, | ||
| diagC, | ||
| nrows | ||
| ); | ||
| solveImplicitTransformComponentDist<2>( | ||
| sys, | ||
| x, | ||
| exec_, | ||
| gkoExec_, | ||
| comm, | ||
| gkoMtx, | ||
| factory_, | ||
| stats, | ||
| l1Control, | ||
| values, | ||
| ma, | ||
| diagC, | ||
| nrows | ||
| ); | ||
| return stats; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
avoid component wise solve here.
greole
force-pushed
the
enh/kOmegaSST
branch
4 times, most recently
from
June 23, 2026 15:38
c9d4a73 to
330e30d
Compare
HendriceH
pushed a commit
that referenced
this pull request
Jun 23, 2026
Cherry-picked from exasim-project/NeoN PR #554 (commit 91c3709). Brings the Ginkgo memory-pool handling: the CUDA executor is now backed by NeoN's Umpire device QuickPool via UmpireCudaAllocator, so Ginkgo's per-solve multigrid build/teardown reuses device memory instead of churning cudaMalloc/cudaFree (which fragmented the device heap and OOM'd the second pressure solve on large cases). Serial/CPU/HIP keep the default executor path. Also carries the upstream parse() fix that type-checks the 'solver' any_cast and remaps solver::Ir's inner preconditioner factory to the 'solver' key Ir::parse() reads. Conflict resolution: the upstream commit also refactored the signatures of solveImplicitTransformComponent / ...Dist (slip-BC implicit-transform ginkgo solvers). Those functions do not exist on this branch — PR #554 introduces them separately and they are unused here — so that hunk was dropped; only the memory-pool handling and the parse() fix are applied. ginkgoDistributed.cpp is therefore unchanged. (cherry picked from commit 91c3709)
greole
force-pushed
the
enh/kOmegaSST
branch
2 times, most recently
from
July 13, 2026 06:02
cfe27e2 to
c5ba297
Compare
…on-linearUpwind runs GeometryScheme eagerly computed and stored two SurfaceField<Vec3> (faceDeltaOwner_, faceDeltaNeighbour_) for every mesh in update(), even when the only consumer (linearUpwind) is not selected. On an ~18M-cell mesh that is ~2*nInternalFaces*Vec3 (~2.6 GB) of always-on device memory, preventing the case from fitting on one GPU. Make them lazy/opt-in, mirroring edf9888 (faceFluxCorrection opt-in): - faceDelta* are now mutable std::optional, allocated+filled only via ensureFaceDeltas(). - LinearUpwind's constructor opts in (while the mesh centres are still alive), so cases using Gauss upwind/linear allocate nothing. - update() no longer computes faceDelta* nor frees the mesh centres; reset() (the centre release) is deferred + made idempotent, triggered on first read of any cached geometry field or right after faceDelta* are built, so a late opt-in can still read the centres. - The fast streaming linearUpwind kernel is unchanged. Runs that select linearUpwind keep identical behaviour and field values.
… linear systems Cache one immutable topology-only bundle per mesh — the CSR system sparsity pattern, its FaceToMatrixAddress, and the boundary COO sparsity — in mesh.stencilDB() (mirroring GeometryScheme::readOrCreate) and share it across every LinearSystem built on that mesh. These arrays depend only on mesh topology and were previously rebuilt and duplicated per equation (U, p, nuTilda, ...); only the per-system value/RHS vectors are now allocated fresh. readOrCreateSparsityBundle is keyed on the sparsity types so the CSR system and COO boundary patterns never collide; the MPI off-diagonal/proc-face sparsity stays per-system (v1). Adds neon_test_sharedSparsity proving pointer sharing and value independence. Pure memory/aliasing refactor — no numerical change.
Unify slip and symmetry onto a shared slipSymmetry helper (they apply the same operator, differing only in registered name and in where they may be applied) and add a normal-damping treatment selectable via the opt-in "implicit" dict key. - Deferred (default): refGrad = -deltaCoeffs*(U.n)*n enters the per-component RHS through the existing fixed-gradient assembly, keeping the shared scalar matrix + multi-RHS solve (zero extra memory). Also feeds grad(U) boundary reconstruction, so no tensor BC is needed there. - Implicit (opt-in): flags BoundaryAttributes::transformImplicit; the Vec3 Laplacian assembly accumulates the per-component diagonal weight g|S|*delta*|n_c| into a lazily-allocated LinearSystem::diagCmpt store. The Ginkgo scalar-matrix/Vec3-rhs solve (serial and distributed) solves the three components segregated, subtracting each column's correction from the shared diagonal in place and restoring it -- no matrix copy. Reuses solve_impl/solve_impl_dist, so the l1ScaledResidual criterion works in both modes. Tests: deferred refGrad + implicit attr (volSlip/volSymmetry), diagCmpt assembly (laplacianOperator), serial segregated solve std+l1 (ginkgo), distributed solveDist std+l1 under MPI_SIZE 3 (distributed/operator).
…3 momentum path The scalar cache (3cbdfadbfb) left the Vec3 momentum solveDist rebuilding its distributed matrix every solve, re-running Csr::create_const's load-balancing srow scan (~72 ms). Wire it to cachedDistMtx_/cachedLocalValPtr_ too: the wrapper is a non-owning VIEW over the rank-local value buffer, so it always reflects live values. Safe for all three sub-paths -- the implicit-transform branch shifts the diagonal in place per component but RESTORES it right after each component solve (atomic_sub / atomic_add pair), the buffer is re-assembled in place each step (pointer stable; the guard rebuilds if it changes), and the fused-slip branch applies its shift through FusedDiagShiftMatrix at the operator level (no buffer mutation). Also fix the createGkoMtxDist signature in test/distributed/operator.cpp (was calling the pre-cache overload). occDrivAer (65M, 4xH200, restart 1050): momentum solve 258 -> 182 ms, steady 0.907 -> 0.830 s/step. Continuity bit-identical (2.79e-6), U/p iters unchanged. Cumulative with the two restored caches: 2.6 -> 0.83 s/step (champion 0.78). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves 13 conflicted files. Most were comment-only, where develop's rewordings were taken to follow its "self-contained comments" policy (54600d6) -- this drops the [fence-audit] tags in vector.cpp and the case-specific debugging notes in inletOutlet.hpp / slipSymmetry.hpp. Substantive resolutions, all taking develop's corrected version: * boundedDiv.cpp explicit path: develop divides by cell volume (sumPhi/V * psi * scaling); the branch had dropped the /V believing the volumes cancel. They cancel only for the implicit Sp diagonal, not for the per-unit-volume explicit result. * ginkgo.cpp / ginkgoDistributed.cpp diagonal transform: develop's plain writes replace the branch's atomics. The parallelFor range is {0, nrows} and each iteration writes its own diagIdx(cell), so there is no race. * gaussGreenDivLaplacian.cpp: bounded_ is now always re-derived, so a second read() with a different scheme cannot leave a stale true. * geometryScheme: develop's eager faceDelta* computation in update() wins over the branch's lazy/opt-in version (c20b8c0). The lazy path had a construction-order dependency -- a consumer built after the first read of another cached geometry field found the mesh centres already freed. This gives up that commit's ~2.6 GB saving on large meshes. * src/CMakeLists.txt: develop's NeoN_SRCS list variable, needed for set_source_files_properties(... LANGUAGE CUDA). The branch's explicit source list was a strict subset, so nothing is lost. The branch's ginkgo work is kept in full: solver caching (Strategy 1b), Workspace reuse (Strategy 3), MergedPgm coarseners, mixed-precision inner solves, sellp local matrix format and the fused slip solve. Develop's l1InConfig_ guard (suppressing a double-applied L1 criterion) was merged into those by hand at all three call sites, since develop applied it to code the branch had rewritten around its solver cache. Ginkgo stays pinned at the branch's ba5e6607, which the MergedPgm code requires; develop's 6a3abf8c does not provide gko::UpdateMatrixValue.
greole
added a commit
that referenced
this pull request
Sep 3, 2026
linearUpwindV is linearUpwind whose deferred gradient correction is built from the cell-limited (minmod, k=1) gradient instead of the unlimited Gauss-Green one, so the extrapolated face value stays inside the value range spanned by the upwind cell and its neighbours. That bound is what keeps convection stable on skewed or refined meshes. Implemented as a `bool CellLimited` template parameter on the existing LinearUpwind, which selects the scheme name and the gradient used by interpolate()/correction(). Instantiated for Vec3 only: cell limiting is a vector-field concept, so the scalar factory deliberately does not offer it. The CellLimitedGrad operator is cached in the per-mesh stencil DB -- it depends only on mesh geometry, so rebuilding it per assemble was a large share of the momentum-assembly cost. GaussGreenDivLaplacian accepts `Gauss linearUpwindV` alongside `Gauss linearUpwind`. Bindings: registered_operator_schemes() now also reports the surface-interpolation factory tables, since a missing scheme aborts the process at lookup rather than raising and so cannot be probed from Python by catching. SurfaceInterpolation gains the flux-aware interpolate_into overload -- without it the upwind-biased schemes are registered but unreachable through the bindings. Tests: linearUpwindV registers for vectors only; it is exact for a linear vector field (limiter inactive); and at a sharp extremum it keeps every face value inside the upwind cell's neighbour range while measurably differing from plain linearUpwind, which proves the cell-limited path is taken. Python tests cover the registration and an end-to-end interpolate. Ported from enh/kOmegaSST (PR #554).
The div/laplacian operators are declared `extern template`, so including their headers in the _neon binding TU does not instantiate them — their self-registration only fires in libNeoN. Compiled `-fvisibility=hidden`, _neon gets a private, empty copy of the runtime-selection factory table, so scheme resolution at assembly time aborts with "Could not find constructor for Gauss". Force explicit instantiation here so the self-registration runs inside _neon and populates its table. Marked with a TODO to drop once the factory table is shared across shared objects.
Drives Expression.read from Python — the call that performs the runtime-selection
factory lookup (create("Gauss")) — to guard the explicit-instantiation fix. Without
that fix the lookup aborts with "Could not find constructor for Gauss". A scalar
equation covers the <scalar> instantiation; a vector equation additionally covers
<Vec3> and <Vec3, scalar> (vector div/laplacian build both same-type and
scalar-matrix strategies).
Adds the minimal bindings the test needs: Expression.read, and
Dictionary.insert_dict / insert_token_list (so a nested divSchemes/laplacianSchemes
dictionary can be built from Python).
Static nanobind gives each extension module its own private nanobind runtime, so types cannot be exchanged across modules reliably (it happens to work when everything is built against one nanobind, but breaks in isolated CI builds). Build _neon against a shared nanobind runtime so it can exchange types with neofoam_bindings and pybFoam (which also uses NB_SHARED) — e.g. so create_adapter_run_time accepts pybFoam's Foam::Time.
NeoN CPM-pinned nanobind 2.9.2 while pybFoam (and pyOFTools) build against pip's 2.13.0. The _neon binding shares pybFoam's libnanobind.so at runtime, but nanobind's ndarray_create signature changed after 2.9.2 (gained a size_t arg), so _neon built against 2.9.2 failed to resolve the symbol from pybFoam's 2.13.0 lib (undefined symbol in CI). Bump to 2.13.0 so the ABIs match; keep it overridable so a downstream can track its own pybFoam. CPM_USE_LOCAL_PACKAGES still reuses an installed nanobind when present.
…ython Add an opt-in NeoN_EXTERNAL_NANOBIND that resolves nanobind via `python -m nanobind --cmake_dir` + find_package instead of fetching NeoN_NANOBIND_VERSION through CPM. Useful together with NeoN_NANOBIND_SHARED: pybFoam is built against the nanobind in the active Python environment, so both modules share one libnanobind ABI by construction rather than by a version pin that has to be kept in sync. Off by default, so CPM with the pinned version stays the default path. The operator-registration fix this branch originally carried (force instantiation of GaussGreenDiv/GaussGreenLaplacian in _neon, NB_SHARED, Expression.read, Dictionary.insert_token_list) landed on develop via #560.
Check registered_operator_schemes() against the expected {Gauss} set for
each div/laplacian factory. Equality makes the expected map the source of
truth, so registration drift (an empty table if self-registration regresses,
or a newly force-instantiated scheme) surfaces immediately without needing a
mesh or a full solver run.
…nstalls
Umpire and its bundled camp are built and installed by their own BLT-based
CMake, which sets each target's INSTALL_RPATH to ${CMAKE_INSTALL_PREFIX}/lib.
Under scikit-build that prefix is the wheel-staging temp directory, which no
longer exists after install, so importing the _neon extension (or any wheel that
bundles NeoN) fails with:
ImportError: libcamp.so: cannot open shared object file: No such file or directory
because libumpire.so cannot find its direct dependency libcamp.so. NeoN's own
libraries are fine ($ORIGIN/../lib) — only these BLT-managed CPM targets escape
NeoN's RPATH policy, because BLT's per-target INSTALL_RPATH wins over the global
CMAKE_INSTALL_RPATH.
Override the umpire/camp targets' INSTALL_RPATH with $ORIGIN-relative entries so
the install is relocatable and libcamp resolves with no LD_LIBRARY_PATH. Guarded
by `if(DEFINED SKBUILD)`, so it only applies to wheel builds and is a no-op for
the C++ build (where the prefix is a real persistent directory and the problem
does not occur).
Claude-Session: https://claude.ai/code/session_01Y4XKvYTbMrxDmBUPVvud4t
…ire and Ginkgo targets to ensure proper runtime linking
linearUpwindV is linearUpwind whose deferred gradient correction is built from the cell-limited (minmod, k=1) gradient instead of the unlimited Gauss-Green one, so the extrapolated face value stays inside the value range spanned by the upwind cell and its neighbours. That bound is what keeps convection stable on skewed or refined meshes. Implemented as a `bool CellLimited` template parameter on the existing LinearUpwind, which selects the scheme name and the gradient used by interpolate()/correction(). Instantiated for Vec3 only: cell limiting is a vector-field concept, so the scalar factory deliberately does not offer it. The CellLimitedGrad operator is cached in the per-mesh stencil DB -- it depends only on mesh geometry, so rebuilding it per assemble was a large share of the momentum-assembly cost. GaussGreenDivLaplacian accepts `Gauss linearUpwindV` alongside `Gauss linearUpwind`. Bindings: registered_operator_schemes() now also reports the surface-interpolation factory tables, since a missing scheme aborts the process at lookup rather than raising and so cannot be probed from Python by catching. SurfaceInterpolation gains the flux-aware interpolate_into overload -- without it the upwind-biased schemes are registered but unreachable through the bindings. Tests: linearUpwindV registers for vectors only; it is exact for a linear vector field (limiter inactive); and at a sharp extremum it keeps every face value inside the upwind cell's neighbour range while measurably differing from plain linearUpwind, which proves the cell-limited path is taken. Python tests cover the registration and an end-to-end interpolate. Ported from enh/kOmegaSST (PR #554).
The docs job installs its packages before checkout with a bare
`sudo apt install`, so it resolves against the package index baked into
the runner image. Ubuntu has since published a newer python3-pil and
dropped the old .deb from the mirror pool, so apt requests a version that
no longer exists and the step dies:
Err:42 ... python3-pil amd64 10.2.0-1ubuntu1.2
404 Not Found
E: Failed to fetch .../python3-pil_10.2.0-1ubuntu1.2_amd64.deb
E: Unable to fetch some archives, maybe run apt-get update ...
Every build_doc run since 2026-09-03 05:52 has failed this way, on
unrelated branches; a re-run cannot help because the file is gone from the
mirror. Adding `apt-get update` makes it resolve against what the mirror
actually carries.
The other install steps already refresh the index, so the remaining change
is hardening: `apt` warns in the logs that its CLI is not stable for
scripting, and without `-y` apt can still stop to ask on a conflict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsge2zsqmNHSmks9eUfCCz
The generated NeoN.hpp umbrella header includes every header unconditionally, so mergedPgm.hpp was pulled into builds configured without Ginkgo and failed on `#include <ginkgo/ginkgo.hpp>`. Wrap it in `#if NF_WITH_GINKGO`, the same guard ginkgo.hpp already uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gsge2zsqmNHSmks9eUfCCz
The three ginkgo patches carried no copyright/licensing information, so the repo-wide `reuse` pre-commit hook failed on every commit. Add the same two-line SPDX header adios2_kokkos.patch already uses. `git apply` skips leading non-diff text, and two of the three patches already started with a Subject line, so the parsed file/hunk set is unchanged. Keep the other hooks off cmake/patches/ while here, since they damage the diffs: - end-of-file-fixer / trailing-whitespace: a context line for a blank source line is a single space, and the final newline belongs to the last hunk. Both fixers strip exactly those. - typos: the bodies are verbatim upstream diffs that must match the third-party source byte for byte, and the headers quote upstream commit SHAs that typos reads as misspellings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gsge2zsqmNHSmks9eUfCCz
DrivAer is the automotive benchmark geometry the occDrivAer case is named after, but `typos` reads the "Aer" as a misspelling of "are" and auto-fixes it. That is how the one remaining occDrivAre in ginkgo.hpp got there. Allow the word, then restore the correct spelling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gsge2zsqmNHSmks9eUfCCz
These three files predate the clang-format hook, so `pre-commit run --all-files` failed on them and CI's static_checks with it. Comment rewrapping and whitespace only -- no behaviour change. Where the formatter's own output read badly, the block is hand-wrapped inside the 100-column limit instead: MergedPgm's doxygen bullets keep their hanging indent, and the two section-separator comments keep their trailing dashes on one line. clang-format leaves both as written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gsge2zsqmNHSmks9eUfCCz
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.
Motivation
WHAT is it, WHY it is needed. 😘