Add optional gcem dependency#62
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds an optional vendored GCE‑Math (gcem) backend selectable at build time, vendors many gcem headers, introduces a math backend header to switch implementations, updates build/CI/make/cmake to support gcem variants, switches many call sites to unqualified math calls, and excludes Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(230,230,255,0.5)
participant Dev as Developer (push/PR)
participant GH as GitHub Actions
participant Matrix as Matrix Expander
participant Runner as Job Runner
end
rect rgba(220,255,220,0.5)
participant CMake as CMake
participant Build as Make/CMake build
participant Tests as Test Runner
end
Dev->>GH: push PR (add gcem, backend, CI/make changes)
GH->>Matrix: expand matrix (compiler × math variant)
Matrix->>Runner: schedule job with math variant env
Runner->>CMake: configure with -DCONSTEIG_USE_GCEM / trait macros
CMake->>CMake: set INTERFACE defs & SYSTEM include for optional_dependencies
Runner->>Build: run conditional make targets (builtin/gcem variants)
Build->>Tests: run tests appropriate for math variant
Tests-->>Runner: report results
Runner-->>GH: upload status/artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Apr 22, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
include/consteig/math/functions/csqrt.hpp (1)
1-6:⚠️ Potential issue | 🟡 MinorAdd explicit includes to restore standalone compile-safety of
csqrt.hpp.
csqrt.hppcalls unqualifiedsqrt()andabs()(lines 28, 36, 41) in a template context. During two-phase lookup, phase-1 (definition-time) visibility is required since the argument types are fundamental (no ADL rescue). Currently,abs.hppandsqrt.hppare not directly included; the file relies onconstmath.hppto include them first (non-GCEM path, lines 40–44) or to bring them into scope viausingdeclarations (GCEM path). While this works in practice because no code in the repo directly includescsqrt.hpp, removing the direct includes makes the file non-standalone and creates a fragile ordering contract.Suggested fix
`#ifndef` CONSTMATH_CSQRT_HPP `#define` CONSTMATH_CSQRT_HPP `#include` "../complex.hpp" +#include "abs.hpp" +#include "sqrt.hpp" `#include` "utilities.hpp"Alternatively, document that
functions/*.hppheaders are internal and onlyconstmath.hppis a supported entry point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/consteig/math/functions/csqrt.hpp` around lines 1 - 6, csqrt.hpp relies on transitive includes for unqualified sqrt() and abs() calls used in template contexts, breaking standalone compile-safety; to fix, add explicit includes for the headers that provide those free functions (include the project's sqrt.hpp and abs.hpp) at the top of csqrt.hpp so the unqualified sqrt()/abs() are visible during phase-1 lookup, leaving the existing code in functions that call sqrt() and abs() unchanged.include/consteig/matrix/operations.hpp (1)
290-304: 🧹 Nitpick | 🔵 TrivialDouble-check that unqualified
sqrtinside the matrixsqrtoverload resolves to the scalar.Both
consteig::sqrt(const Matrix&)and the scalarconsteig::sqrtlive in the same namespace, so unqualified lookup sees both overloads and picks the scalar format(row, col)— correct today. Just flagging that if a future change ever makesMatrix<T,R,C>implicitly convertible fromT(or introduces aMatrix<Matrix<...>>-style nesting), this becomes an infinite-recursion trap. Aconstexprstatic check or simply qualifying the inner call asconsteig::sqrt(mat(row, col))via a tiny helper would harden it. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/consteig/matrix/operations.hpp` around lines 290 - 304, In the Matrix overload sqrt(const Matrix<T,R,C>&) the unqualified call sqrt(mat(row, col)) can later resolve to the matrix overload itself; change the inner call to the scalar sqrt by qualifying it (e.g. use ::std::sqrt(mat(row, col)) and include <cmath>), so the body of the function sqrt(const Matrix<...>) always invokes the scalar sqrt on each element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CMakeLists.txt`:
- Around line 54-65: Declare CONSTEIG_GCEM_USE_STDLIB only when
CONSTEIG_USE_GCEM is enabled and make the vendored gcem include path a SYSTEM
include on the consteig target: move the option(CONSTEIG_GCEM_USE_STDLIB ...)
inside the if(CONSTEIG_USE_GCEM) guard so it cannot be set as a no-op, and when
enabling GCEM add a SYSTEM include for the vendored subtree (use
target_include_directories(consteig INTERFACE SYSTEM
"include/consteig/optional_dependencies/gcem")) so third‑party headers aren’t
counted against consumer warning/error budgets; keep the existing
target_compile_definitions(consteig INTERFACE CONSTEIG_USE_GCEM) and conditional
CONSTEIG_GCEM_USE_STDLIB definition.
In `@include/consteig/math/complex.hpp`:
- Around line 4-6: The abs(const Complex<T>&) template calls an unqualified sqrt
in complex.hpp but doesn't ensure sqrt is declared at the definition point; make
complex.hpp self-sufficient by adding an include for the sqrt implementation
header (functions/sqrt.hpp) so that sqrt is visible when abs (and any other
dependent calls) are instantiated; update include/consteig/math/complex.hpp to
`#include` "functions/sqrt.hpp" (keeping the existing call in abs and the
Complex<T> type unchanged).
In `@include/consteig/math/constmath.hpp`:
- Around line 6-13: The preprocessor checks in
include/consteig/math/constmath.hpp are fragile: replace the conditional macro
checks so they are value-based (change `#ifdef` CONSTEIG_USE_GCEM to `#if`
CONSTEIG_USE_GCEM and change `#ifndef` CONSTEIG_GCEM_USE_STDLIB to `#if`
!CONSTEIG_GCEM_USE_STDLIB) and add numeric default definitions for these macros
in include/consteig/consteig_options.hpp (e.g., define CONSTEIG_USE_GCEM and
CONSTEIG_GCEM_USE_STDLIB to 0/1 as appropriate) so passing -D...=0 behaves
correctly; update references to the macros CONSTEIG_USE_GCEM and
CONSTEIG_GCEM_USE_STDLIB in constmath.hpp to match the new value-based checks.
In `@include/consteig/math/functions/complex_exp.hpp`:
- Around line 19-23: The header complex_exp.hpp defines template Complex<T>
exp(const Complex<T>&) and currently assumes scalar exp(T), cos(T), sin(T) are
visible; add a structural safeguard by either explicitly including the scalar
headers (e.g., include the headers that define exp(T) and trig functions used by
exp(z.real), cos(z.imag), sin(z.imag)) at the top of this file, or add a clear
compile-time check after including complex.hpp that emits a preprocessor error
if constmath.hpp wasn't included (e.g., `#error` "include constmath.hpp instead"),
so that the symbols used by template Complex<T> exp(...) are guaranteed to be
available when complex_exp.hpp is included.
In `@include/consteig/math/functions/utilities.hpp`:
- Around line 68-74: The public utility header removed the definition of the
constexpr helper equalWithin leaving only a declaration in utilities.hpp so
users can call it but may fail to link/compile because the definition lives
later in matrix/operations.hpp; restore the inline constexpr definition of
equalWithin into utilities.hpp (so it remains header-only and self-contained),
ensuring it uses the appropriate std::abs or backend-agnostic abs and keeps the
same template signature (template<typename T, typename U> constexpr bool
equalWithin(T a, T b, U thresh)) so matrix/operations.hpp and downstream callers
can include utilities.hpp without depending on later definitions.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/atan2.hpp`:
- Around line 58-85: The public wrapper atan2 currently calls
internal::atan2_type_check(x,y) which swaps the arguments again; change the call
in atan2 to internal::atan2_type_check(y, x) so the argument order matches the
atan2 signature and the internal function's parameters (ensure atan2_type_check
and atan2_compute keep the natural (y,x) order to avoid accidental swaps).
In `@include/consteig/optional_dependencies/gcem/gcem_incl/erf_inv.hpp`:
- Around line 198-205: erf_inv_decision currently advances the iterate to value
- direc but calls erf_inv_recur with erf_inv_deriv_1(value) (the derivative at
the old value); recompute the derivative at the new iterate so Halley steps use
f'(x_n) not f'(x_{n-1}). Change the call in erf_inv_decision that recurses to
erf_inv_recur(value - direc, p, erf_inv_deriv_1(value - direc), iter_count+1) so
the derivative passed matches the new point; keep the else branch behavior
otherwise.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/find_exponent.hpp`:
- Around line 43-58: The recursive find_exponent function currently checks the x
>= T(10) branch before the x > T(1e+02) and x > T(1e+04) branches, making the
/100 and /10000 branches unreachable for large inputs and causing excessive
single-decade recursion; fix it by reordering the large-value branches in
find_exponent so you test the coarser scales first (check x > T(1e+04) then x >
T(1e+02) then x >= T(10)) and keep their corresponding recursive calls (divide
by T(1e+04), T(1e+02), T(10) and add llint_t(4), llint_t(2), llint_t(1) to
exponent) so large x values reduce by larger factors per recursion.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/gcem_type_traits.hpp`:
- Line 49: Add a traditional include guard around the entire header file
gcem_type_traits.hpp to prevent multiple re-inclusions (wrap the file with
`#ifndef` / `#define` / `#endif` using a unique macro name consistent with other GCEM
headers, e.g. GCEM_TYPE_TRAITS_HPP or GCEM_GCEM_TYPE_TRAITS_HPP); ensure the
guard encloses everything including the existing `#include` "gcem_utility.hpp" and
all template/specialization definitions so re-including this header in
GCEM_TRAITS_BUILTIN mode will no longer produce redefinition errors.
In
`@include/consteig/optional_dependencies/gcem/gcem_incl/incomplete_beta_inv.hpp`:
- Around line 274-278: The Halley recursion is using a stale derivative: in
incomplete_beta_inv_recur(...) you pass
incomplete_beta_inv_deriv_1(value,alpha_par,beta_par,lb_val) while the next
iterate is value - direc; recompute the derivative at the updated point and pass
incomplete_beta_inv_deriv_1(value - direc, alpha_par, beta_par, lb_val) (i.e.,
call incomplete_beta_inv_deriv_1 with value-direc) so the derivative matches the
residual used in the next incomplete_beta_inv_recur invocation.
- Around line 300-310: Add explicit domain validation before the
boundary-shortcut checks: after the existing any_nan(...) NaN check and before
the "indistinguishable from zero or one" comparisons, verify that p is within
[0,1] and that alpha_par>0 and beta_par>0; if any of these fail return
GCLIM<T>::quiet_NaN(). In other words, inside the same return expression that
currently branches to T(0)/T(1)/incomplete_beta_inv_begin, insert a guard that
rejects p<0, p>1, alpha_par<=0 or beta_par<=0 (use the same T/alpha_par/beta_par
symbols) so invalid inputs cannot flow into lbeta(...) or the root/iteration
routines (incomplete_beta_inv_initial_val, incomplete_beta_inv_begin).
In `@include/consteig/optional_dependencies/gcem/gcem_incl/incomplete_beta.hpp`:
- Around line 141-150: The current return expression in incomplete_beta.hpp can
short-circuit negatives and invalid parameter values into unsafe paths; add
explicit domain checks to reject invalid inputs (a <= 0, b <= 0, z < 0, z > 1)
by returning GCLIM<T>::quiet_NaN() before any other shortcuts, and add an
explicit near-one shortcut symmetric to the existing near-zero shortcut (i.e.
test if 1 - z is below the underflow threshold and return T(1) directly) so
calls to incomplete_beta_begin(a,b,z) and incomplete_beta_begin(b,a,T(1)-z)
never receive invalid or numerically unstable arguments; update the logic around
any_nan(a,b,z), the zero-shortcut test using GCLIM<T>::min() > z, and the final
branch that calls incomplete_beta_begin to enforce these guards.
In
`@include/consteig/optional_dependencies/gcem/gcem_incl/incomplete_gamma_inv.hpp`:
- Around line 188-198: The decision branch in incomplete_gamma_inv_decision
calls incomplete_gamma_inv_recur for the next iterate but passes
incomplete_gamma_inv_deriv_1(value, a, lg_val) (derivative at the old point);
change this to recompute the derivative at the new iterate by passing
incomplete_gamma_inv_deriv_1(value - direc, a, lg_val) to
incomplete_gamma_inv_recur (preserving iter_count+1 and the existing
max-iteration check in incomplete_gamma_inv_decision and using the same
parameters for incomplete_gamma_inv_recur).
- Around line 217-231: The function incomplete_gamma_inv currently returns 0 for
negative a/p due to the near-zero shortcuts; add an explicit domain check so
negative-domain inputs yield NaN like p>1 does: after the any_nan(a,p) check,
test if a < T(0) || p < T(0) and return GCLIM<T>::quiet_NaN() when true; leave
the other branches (p> T(1), small-p, small-|1-p|, small-a) unchanged and then
call incomplete_gamma_inv_begin(incomplete_gamma_inv_initial_val(a,p), a, p,
lgamma(a)) for the valid case.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/incomplete_gamma.hpp`:
- Around line 190-208: The code currently returns 0 for negative z due to the
GCLIM<T>::min() > z check; update the ternary chain in the incomplete gamma
return to reject negative z like negative a by checking z < T(0) (or similar)
and returning GCLIM<T>::quiet_NaN() before the near-zero shortcut that returns
T(0); modify the sequence of conditions around any_nan(a,z), a < T(0), and
GCLIM<T>::min() > z so negative z is handled with NaN (refer to the ternary
expression and symbols incomplete_gamma_cf_1, incomplete_gamma_cf_2,
incomplete_gamma_quad).
In `@include/consteig/optional_dependencies/gcem/gcem_incl/inv_sqrt.hpp`:
- Around line 61-66: The branch that seeds Newton with x/T(2) (inside
inv_sqrt(...) calling inv_sqrt_recur) is not scale-safe for very large or very
small finite nonzero x; change this branch to first normalize/rescale x into a
safe range (e.g., extract exponent, scale x to [min_safe, max_safe], call
inv_sqrt_recur on the scaled value) and then undo the scale on the result, or
simply route the case to a scale-safe sqrt-based path that computes 1/sqrt(x)
with exponent-aware scaling; update inv_sqrt, the branch that currently does
GCLIM<T>::min() > abs(T(1) - x) ? x : inv_sqrt_recur(x, x/T(2), 0), and any
helper that seeds Newton so they perform exponent normalization (or delegate to
the stable sqrt path) for large/small finite nonzero inputs to avoid
overflow/underflow in x * xn.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/lbeta.hpp`:
- Around line 33-40: The lbeta template performs a+b in the original integer
type which can overflow; convert/promote the inputs to the common_return_t
before any arithmetic so lgamma and the sum operate on the promoted type: inside
lbeta (the template named lbeta with params a,b and return type
common_return_t<T1,T2>) create promoted variables like common_return_t<T1,T2> aa
= a, bb = b and then compute return (lgamma(aa) + lgamma(bb)) - lgamma(aa + bb)
so the addition uses the wider type.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/lmgamma.hpp`:
- Around line 39-49: The lmgamma branch recurses using p without guarding
against NaN/infinite T2 values (so lmgamma(a, NaN) never matches p==T2(1) or
p<T2(1) and recurses forever); update the lmgamma implementation (the return
expression using lmgamma_recur) to first check p with is_nan(p) or !is_finite(p)
(similar to the is_nan(a) check) and return GCLIM<T1>::quiet_NaN() when p is not
a valid number, before the p==T2(1) / p<T2(1) tests and before calling
lmgamma_recur, ensuring lmgamma and lmgamma_recur both handle invalid p safely.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/mantissa.hpp`:
- Around line 39-46: The mantissa function currently rescales only by a factor
of 10 per recursion which can hit constexpr recursion limits for very large
long-double inputs; update mantissa(...) to mirror find_exponent by adding
coarse scaling steps (check x >= T(1e4) then x >= T(1e2) before the existing x
>= T(10) branch, and optionally mirror the small-value multipliers x < T(1e-4)
and x < T(1e-2) before the x < T(1) branch) so values are divided/multiplied by
1e4/1e2 (and 1e-4/1e-2) to reduce recursion depth while preserving the existing
logic and epsilon checks.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/neg_zero.hpp`:
- Around line 25-37: The header defines the template internal::neg_zero which
can be redefined if the header is included twice; add a traditional include
guard around the entire file (e.g., define a unique macro like
GCEM_INCL_NEG_ZERO_HPP or GCEM_NEG_ZERO_HPP) so the contents (namespace internal
{ template<typename T> constexpr bool neg_zero(const T x) noexcept { ... } })
are only processed once, placing the `#ifndef/`#define at the top and a matching
`#endif` at the end of the file.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/pow_integral.hpp`:
- Around line 87-90: The current branches return exponent-limit sentinels
unconditionally (exp_term == GCLIM<T2>::min()/max()) which yields incorrect
results for bases like 1 or -1; update pow_integral so that when exp_term ==
GCLIM<T2>::max() you do NOT return GCLIM<T1>::infinity() and instead fall
through to the existing squaring/loop path (let the normal exponentiation handle
max), and when exp_term == GCLIM<T2>::min() only return the sentinel after first
checking base semantics (handle base == T1(1) and base == T1(-1) to return
finite results, and handle the zero/other-base cases explicitly) — modify the
conditional around exp_term (the branches referencing GCLIM<T2>::min() and
GCLIM<T2>::max() in pow_integral) to perform these base checks rather than
returning sentinels immediately.
In `@include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp`:
- Around line 74-92: tanh_check can call tanh_begin which squares its input and
can overflow for large finite x; before calling tanh_begin(x) or tanh_begin(-x)
add a guard that if abs(x) >= sqrt_max (where sqrt_max = sqrt(GCLIM<T>::max())
computed in a constexpr-safe way) return GCLIM<T>::max() sign-normalized to ±1
(i.e. T(1) or T(-1)) instead of calling tanh_begin; update tanh_check to compute
sqrt_max once and use it to saturate large magnitudes prior to the tanh_begin
calls.
In `@scripts/vendor_gcem.sh`:
- Around line 32-36: Make the rm step defensive by using the parameter-expansion
guard for DEST so an unset DEST cannot make rm -rf target the repo root (change
the rm invocation that uses "$REPO_ROOT/$DEST" to use ${DEST:?} and keep mkdir
-p consistent), and add an inline comment next to the git clone invocation (the
line that uses REF in git clone --depth=1 --branch "$REF") stating that REF must
be a branch or tag (commit SHAs are not accepted by --branch) or else adjust the
clone logic to support checking out a SHA; update references to DEST, REPO_ROOT
and REF in the script accordingly.
In `@tests/CMakeLists.txt`:
- Around line 1-3: Add a brief explanatory comment above the conditional that
skips adding the math test subdirectory when CONSTEIG_USE_GCEM is ON: explain
that math unit tests are skipped because GCEM's results differ by the last ULP
versus consteig's hand‑rolled implementations (causing the 1e-9 tolerance
assertions to fail), and note that GCEM is still indirectly covered by matrix
and eigen tests which exercise functions like abs() and sqrt(); keep the
conditional using add_subdirectory(math) unchanged to avoid breaking GCEM CI.
---
Outside diff comments:
In `@include/consteig/math/functions/csqrt.hpp`:
- Around line 1-6: csqrt.hpp relies on transitive includes for unqualified
sqrt() and abs() calls used in template contexts, breaking standalone
compile-safety; to fix, add explicit includes for the headers that provide those
free functions (include the project's sqrt.hpp and abs.hpp) at the top of
csqrt.hpp so the unqualified sqrt()/abs() are visible during phase-1 lookup,
leaving the existing code in functions that call sqrt() and abs() unchanged.
In `@include/consteig/matrix/operations.hpp`:
- Around line 290-304: In the Matrix overload sqrt(const Matrix<T,R,C>&) the
unqualified call sqrt(mat(row, col)) can later resolve to the matrix overload
itself; change the inner call to the scalar sqrt by qualifying it (e.g. use
::std::sqrt(mat(row, col)) and include <cmath>), so the body of the function
sqrt(const Matrix<...>) always invokes the scalar sqrt on each element.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1bf00d43-0d13-4e64-9707-4140f4461711
📒 Files selected for processing (88)
.coderabbit.yaml.github/workflows/main.yml.gitignoreCMakeLists.txtMakefileinclude/consteig/consteig_options.hppinclude/consteig/eigen/eigen.hppinclude/consteig/math/complex.hppinclude/consteig/math/constmath.hppinclude/consteig/math/functions/complex_exp.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/exp.hppinclude/consteig/math/functions/utilities.hppinclude/consteig/matrix/decompositions/householder.hppinclude/consteig/matrix/decompositions/lu.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/operations.hppinclude/consteig/optional_dependencies/gcem/gcem.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/abs.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/acos.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/acosh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/asin.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/asinh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/atan.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/atan2.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/atanh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/beta.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/binomial_coef.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/ceil.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/copysign.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/cos.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/cosh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/erf.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/erf_inv.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/exp.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/expm1.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/fabs.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/fabsf.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/fabsl.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/factorial.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/find_exponent.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/find_fraction.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/find_whole.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/floor.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/fmod.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/gcd.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/gcem_options.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/gcem_type_traits.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/hypot.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/incomplete_beta.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/incomplete_beta_inv.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/incomplete_gamma.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/incomplete_gamma_inv.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/inv_sqrt.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/is_even.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/is_finite.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/is_inf.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/is_nan.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/is_odd.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/lbeta.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/lcm.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/lgamma.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/lmgamma.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/log.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/log10.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/log1p.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/log2.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/log_binomial_coef.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/mantissa.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/max.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/min.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/neg_zero.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/pow.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/pow_integral.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/quadrature/gauss_legendre_30.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/quadrature/gauss_legendre_50.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/round.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/sgn.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/signbit.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/sin.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/sinh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/sqrt.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/tan.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/tanh.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/tgamma.hppinclude/consteig/optional_dependencies/gcem/gcem_incl/trunc.hppscripts/vendor_gcem.shtests/CMakeLists.txt
…ssue for complex exp
There was a problem hiding this comment.
♻️ Duplicate comments (2)
include/consteig/math/complex.hpp (1)
4-6:⚠️ Potential issue | 🟡 MinorMake
complex.hppself-sufficient for the scalar math it calls.
abs(Complex<T>)and the newexp(Complex<T>)call unqualifiedsqrt,exp,cos, andsin, but this header only gets those declarations when included throughconstmath.hppin the right order. Direct inclusion ofcomplex.hppcan still fail under C++ two-phase lookup; the new complexexpextends the same issue beyondsqrt.One possible backend-aware fix
`#include` "../consteig_options.hpp" +#if CONSTEIG_USE_GCEM +#if !CONSTEIG_GCEM_USE_STDLIB && !defined(GCEM_TRAITS_BUILTIN) +#define GCEM_TRAITS_BUILTIN +#endif +#include "../optional_dependencies/gcem/gcem.hpp" +#else +#include "functions/exp.hpp" +#include "functions/sqrt.hpp" +#include "functions/trig.hpp" +#endif + `#include` "functions/utilities.hpp" namespace consteig { + +#if CONSTEIG_USE_GCEM +using gcem::cos; +using gcem::exp; +using gcem::sin; +using gcem::sqrt; +#endifVerify the include-order dependency with:
#!/bin/bash # Description: Check whether complex.hpp declares/includes the scalar math names it uses. # Expected: complex.hpp should provide declarations for sqrt/exp/cos/sin before the templates that call them. sed -n '1,20p;132,151p' include/consteig/math/complex.hpp printf '\n--- Scalar math declarations/includes in complex.hpp ---\n' rg -n '#include "functions/(sqrt|exp|trig)\.hpp"|using gcem::(sqrt|exp|cos|sin)' include/consteig/math/complex.hpp || true printf '\n--- Declarations currently supplied by constmath.hpp include order ---\n' sed -n '1,55p' include/consteig/math/constmath.hppAs per coding guidelines, “All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.”
Also applies to: 136-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/consteig/math/complex.hpp` around lines 4 - 6, complex.hpp is not self-sufficient: templates like abs(Complex<T>) and exp(Complex<T>) call unqualified sqrt/exp/cos/sin but rely on include order from constmath.hpp; fix by making complex.hpp provide the scalar math declarations it uses (either `#include` the backend-agnostic headers such as "functions/sqrt.hpp", "functions/exp.hpp" and the trig header providing cos/sin, or add explicit using-declarations like using gcem::sqrt; using gcem::exp; using gcem::cos; using gcem::sin;) so that functions abs(Complex<T>) and exp(Complex<T>) resolve correctly under two-phase lookup and direct inclusion.include/consteig/math/constmath.hpp (1)
6-13:⚠️ Potential issue | 🟠 MajorUse value-based checks for option macros.
#ifdef CONSTEIG_USE_GCEMtreats-DCONSTEIG_USE_GCEM=0as enabled, and#ifndef CONSTEIG_GCEM_USE_STDLIBtreats-DCONSTEIG_GCEM_USE_STDLIB=0as enabled stdlib mode. Use numeric defaults plus#ifchecks so OFF values behave as OFF.Proposed fix
-#ifdef CONSTEIG_USE_GCEM +#if CONSTEIG_USE_GCEM // Configure gcem's type-traits mode BEFORE including any gcem header. // Without CONSTEIG_GCEM_USE_STDLIB, gcem uses compiler builtins only // (freestanding). With it, gcem uses <limits> and <type_traits>. -#ifndef CONSTEIG_GCEM_USE_STDLIB +#if !CONSTEIG_GCEM_USE_STDLIB `#define` GCEM_TRAITS_BUILTIN `#endif`Also ensure
include/consteig/consteig_options.hppprovides numeric defaults:+#ifndef CONSTEIG_USE_GCEM +#define CONSTEIG_USE_GCEM 0 +#endif + +#ifndef CONSTEIG_GCEM_USE_STDLIB +#define CONSTEIG_GCEM_USE_STDLIB 0 +#endifVerify the current wiring with:
#!/bin/bash # Description: Inspect option macro checks and defaults. # Expected: option checks should be value-based (`#if`), and defaults should define numeric 0/1 values. rg -n -C3 'CONSTEIG_USE_GCEM|CONSTEIG_GCEM_USE_STDLIB' include/consteig/consteig_options.hpp include/consteig/math/constmath.hpp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/consteig/math/constmath.hpp` around lines 6 - 13, Replace value-less macro checks with numeric/value-based checks: change the top-level guard from `#ifdef CONSTEIG_USE_GCEM` to `#if CONSTEIG_USE_GCEM` and change the nested check from `#ifndef CONSTEIG_GCEM_USE_STDLIB` to `#if !CONSTEIG_GCEM_USE_STDLIB` so a user passing `-DCONSTEIG_USE_GCEM=0` or `-DCONSTEIG_GCEM_USE_STDLIB=0` behaves as OFF; also ensure the options header defines numeric defaults (e.g., `#ifndef CONSTEIG_USE_GCEM `#define` CONSTEIG_USE_GCEM 0 `#endif`` and similarly for `CONSTEIG_GCEM_USE_STDLIB`) so the `#if` checks are well-defined, and keep the existing action of defining `GCEM_TRAITS_BUILTIN` when stdlib mode is disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@include/consteig/math/complex.hpp`:
- Around line 4-6: complex.hpp is not self-sufficient: templates like
abs(Complex<T>) and exp(Complex<T>) call unqualified sqrt/exp/cos/sin but rely
on include order from constmath.hpp; fix by making complex.hpp provide the
scalar math declarations it uses (either `#include` the backend-agnostic headers
such as "functions/sqrt.hpp", "functions/exp.hpp" and the trig header providing
cos/sin, or add explicit using-declarations like using gcem::sqrt; using
gcem::exp; using gcem::cos; using gcem::sin;) so that functions abs(Complex<T>)
and exp(Complex<T>) resolve correctly under two-phase lookup and direct
inclusion.
In `@include/consteig/math/constmath.hpp`:
- Around line 6-13: Replace value-less macro checks with numeric/value-based
checks: change the top-level guard from `#ifdef CONSTEIG_USE_GCEM` to `#if
CONSTEIG_USE_GCEM` and change the nested check from `#ifndef
CONSTEIG_GCEM_USE_STDLIB` to `#if !CONSTEIG_GCEM_USE_STDLIB` so a user passing
`-DCONSTEIG_USE_GCEM=0` or `-DCONSTEIG_GCEM_USE_STDLIB=0` behaves as OFF; also
ensure the options header defines numeric defaults (e.g., `#ifndef
CONSTEIG_USE_GCEM `#define` CONSTEIG_USE_GCEM 0 `#endif`` and similarly for
`CONSTEIG_GCEM_USE_STDLIB`) so the `#if` checks are well-defined, and keep the
existing action of defining `GCEM_TRAITS_BUILTIN` when stdlib mode is disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fe693be0-ed07-49c9-a468-5c928ae7c537
⛔ Files ignored due to path filters (2)
include/consteig/optional_dependencies/gcem/gcem_incl/gcem_limits.hppis excluded by!**/optional_dependencies/**include/consteig/optional_dependencies/gcem/gcem_incl/gcem_utility.hppis excluded by!**/optional_dependencies/**
📒 Files selected for processing (2)
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (AGENT.md)
Always use braces for all control flow bodies (if, else, for, while), even single-line ones. No braceless statements.
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
include/consteig/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
**/*.hpp
⚙️ CodeRabbit configuration file
**/*.hpp: This is a constexpr C++17 header-only library. Focus on:
- Correctness of constexpr computations and static_assert usage
- Type consistency (Matrix<float,...> must use float tolerances, Matrix<double,...> must use double)
- Explicit casts when indexing Eigen matrices with consteig::Size (unsigned → signed narrowing)
- Tolerance constants are defined in test_dependencies/test_tools.hpp — never hardcode tolerance values, and use the constant appropriate for the method type (direct, iterative, pathological, large magnitude)
- No standard library dependencies in core library code (freestanding constraint)
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
🧠 Learnings (23)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:25.802Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:25.802Z
Learning: In production headers under the math/, matrix/, or eigen/ directories, do not introduce or include standard library headers. This guideline prevents std dependency in these header-only/zero-dependency components. If you need NaN or similar constants, use compiler built-ins that are constexpr-compatible (e.g., __builtin_nan(""), __builtin_nanf(""), __builtin_nanl("")); note that the empty string argument is required for constexpr support, and non-empty strings like "nan" are not constant expressions. Apply this guidance to any production headers in these directories; tests may diverge and still use standard headers there.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to include/consteig/**/*.{cpp,hpp} : All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:58:46.252Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-27T19:12:21.039Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: math/functions/pow.hpp:24-36
Timestamp: 2026-03-27T19:12:21.039Z
Learning: In `math/functions/pow.hpp` (MitchellThompkins/consteig), the signed-exponent overload `template <typename T> constexpr T pow(const T x, const int n)` intentionally accepts integral T with positive n (e.g., `pow<int>(x, 3)`). `n` is a runtime parameter so `static_assert(n >= 0)` is not a valid compile-time guard. There is no clean way to restrict negative exponents to floating-point types without changing the API (e.g., using a non-type template parameter). Do not suggest static_assert or if-constexpr guards on runtime function parameters.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE for double matrices in tests of iterative methods (Hessenberg, QR iteration).
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:52:20.667Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-14T07:47:11.893Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/sqrt.hpp:0-0
Timestamp: 2026-03-14T07:47:11.893Z
Learning: In math/functions/utilities.hpp (consteig project), the final design for poison_nan<T>() and force_compile_time_error_for_negative_sqrt<T>() distinguishes between types: for floating-point types, the poison value is static_cast<T>(0.0) / static_cast<T>(0.0) (true IEEE 754 NaN), and is_poison_nan uses x != x; for integer types, the poison value is static_cast<T>(-1) and is_poison_nan checks x == -1. The non-constexpr nature of force_compile_time_error_for_negative_sqrt ensures compile-time evaluation of sqrt(negative) is a hard build error, while runtime returns a meaningful poison value per type. Do not revert this type-discriminated behavior.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use PATHOLOGICAL_TOL for tests involving defective or ill-conditioned matrices to account for theoretical accuracy limitations.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T05:08:21.606Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:21.606Z
Learning: In math/functions/utilities.hpp of the consteig project, the NaN generation via static_cast<T>(0.0) / static_cast<T>(0.0) is an intentional design choice: it yields NaN at runtime on IEEE 754 platforms, but triggers a compile-time error (UB in constant expressions) in constexpr evaluation. Treat this as acceptable for a compile-time-first library: attempting to compute sqrt(negative) at compile time should be an error, and a separate constexpr path (csqrt()) should be used for negative inputs in constexpr contexts. There is no portable, std-free, ifdef-free, constexpr-NaN that works across GCC, Clang, and MSVC; this approach is the best given the project constraints. When reviewing, preserve this rationale and ensure any refactoring maintaining the same dual behavior, and document constraints about compiler support if modifying NaN generation mechanisms.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-25T07:42:50.869Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 46
File: matrix/tests/matrix.test.cpp:442-445
Timestamp: 2026-03-25T07:42:50.869Z
Learning: In `matrix/tests/matrix.test.cpp` (MitchellThompkins/consteig), use `ASSERT_EQ` (not `EXPECT_NEAR`) for runtime checks in `make_matrix` tests. The function merely stores provided literal values into the matrix with no floating-point arithmetic, so the retrieved values are bit-for-bit identical to the inputs — exact equality is correct and `EXPECT_NEAR` with a tolerance constant is not needed.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_ITERATIVE_FLOAT_TOLERANCE for float matrices in tests of iterative methods (Hessenberg, QR iteration) to account for error accumulation.
Applied to files:
include/consteig/math/constmath.hpp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/consteig/math/constmath.hpp`:
- Around line 23-30: Add a short explanatory comment above the block of
using-declarations (the lines with using gcem::abs; using gcem::sqrt; etc.)
stating that this is an intentional re-export boundary to allow unqualified
calls (e.g. sqrt(...) at line 63, exp/cos/sin at 74-75) to resolve to GCEM, and
warn maintainers not to add additional using gcem::... entries for symbols that
have local Complex<T> overloads (like abs(Complex<T>), exp(Complex<T>)) because
doing so could silently break overload resolution in the future; place the
comment inside the consteig namespace near the existing using statements so the
intent is clear to future editors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f05c1442-749e-4719-9cdf-01e4db056721
📒 Files selected for processing (2)
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: gcc-test-gcem
- GitHub Check: gcc-test-gcem-stdlib
- GitHub Check: gcc-test
- GitHub Check: clang-test-gcem-stdlib
- GitHub Check: clang-test
- GitHub Check: clang-test-gcem
- GitHub Check: cross-arm-gcc
- GitHub Check: cross-arm-clang-gcem
- GitHub Check: cross-arm-gcc-gcem
- GitHub Check: cross-arm-clang
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (AGENT.md)
Always use braces for all control flow bodies (if, else, for, while), even single-line ones. No braceless statements.
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
include/consteig/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
**/*.hpp
⚙️ CodeRabbit configuration file
**/*.hpp: This is a constexpr C++17 header-only library. Focus on:
- Correctness of constexpr computations and static_assert usage
- Type consistency (Matrix<float,...> must use float tolerances, Matrix<double,...> must use double)
- Explicit casts when indexing Eigen matrices with consteig::Size (unsigned → signed narrowing)
- Tolerance constants are defined in test_dependencies/test_tools.hpp — never hardcode tolerance values, and use the constant appropriate for the method type (direct, iterative, pathological, large magnitude)
- No standard library dependencies in core library code (freestanding constraint)
Files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:25.802Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:25.802Z
Learning: In production headers under the math/, matrix/, or eigen/ directories, do not introduce or include standard library headers. This guideline prevents std dependency in these header-only/zero-dependency components. If you need NaN or similar constants, use compiler built-ins that are constexpr-compatible (e.g., __builtin_nan(""), __builtin_nanf(""), __builtin_nanl("")); note that the empty string argument is required for constexpr support, and non-empty strings like "nan" are not constant expressions. Apply this guidance to any production headers in these directories; tests may diverge and still use standard headers there.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:58:46.252Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to include/consteig/**/*.{cpp,hpp} : All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:52:20.667Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T07:47:11.893Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/sqrt.hpp:0-0
Timestamp: 2026-03-14T07:47:11.893Z
Learning: In math/functions/utilities.hpp (consteig project), the final design for poison_nan<T>() and force_compile_time_error_for_negative_sqrt<T>() distinguishes between types: for floating-point types, the poison value is static_cast<T>(0.0) / static_cast<T>(0.0) (true IEEE 754 NaN), and is_poison_nan uses x != x; for integer types, the poison value is static_cast<T>(-1) and is_poison_nan checks x == -1. The non-constexpr nature of force_compile_time_error_for_negative_sqrt ensures compile-time evaluation of sqrt(negative) is a hard build error, while runtime returns a meaningful poison value per type. Do not revert this type-discriminated behavior.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use PATHOLOGICAL_TOL for tests involving defective or ill-conditioned matrices to account for theoretical accuracy limitations.
Applied to files:
include/consteig/math/complex.hpp
📚 Learning: 2026-03-14T05:08:21.606Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:21.606Z
Learning: In math/functions/utilities.hpp of the consteig project, the NaN generation via static_cast<T>(0.0) / static_cast<T>(0.0) is an intentional design choice: it yields NaN at runtime on IEEE 754 platforms, but triggers a compile-time error (UB in constant expressions) in constexpr evaluation. Treat this as acceptable for a compile-time-first library: attempting to compute sqrt(negative) at compile time should be an error, and a separate constexpr path (csqrt()) should be used for negative inputs in constexpr contexts. There is no portable, std-free, ifdef-free, constexpr-NaN that works across GCC, Clang, and MSVC; this approach is the best given the project constraints. When reviewing, preserve this rationale and ensure any refactoring maintaining the same dual behavior, and document constraints about compiler support if modifying NaN generation mechanisms.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
include/consteig/math/complex.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE for double matrices in tests of iterative methods (Hessenberg, QR iteration).
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-25T07:42:50.869Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 46
File: matrix/tests/matrix.test.cpp:442-445
Timestamp: 2026-03-25T07:42:50.869Z
Learning: In `matrix/tests/matrix.test.cpp` (MitchellThompkins/consteig), use `ASSERT_EQ` (not `EXPECT_NEAR`) for runtime checks in `make_matrix` tests. The function merely stores provided literal values into the matrix with no floating-point arithmetic, so the retrieved values are bit-for-bit identical to the inputs — exact equality is correct and `EXPECT_NEAR` with a tolerance constant is not needed.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_ITERATIVE_FLOAT_TOLERANCE for float matrices in tests of iterative methods (Hessenberg, QR iteration) to account for error accumulation.
Applied to files:
include/consteig/math/constmath.hpp
🔇 Additional comments (3)
include/consteig/math/constmath.hpp (2)
6-13: Prefer value-based macro checks over#ifdef/#ifndefto avoid-D...=0footguns.
#ifdef CONSTEIG_USE_GCEMis true even when the user builds with-DCONSTEIG_USE_GCEM=0, and symmetrically#ifndef CONSTEIG_GCEM_USE_STDLIBis false whenever the macro is defined to any value (including0). That makes "off" configurations surprisingly turn features on. Switching to value-based checks plus numeric defaults inconsteig_options.hppkeeps-D...=0honest.♻️ Proposed fix
-#ifdef CONSTEIG_USE_GCEM +#if CONSTEIG_USE_GCEM // Configure gcem's type-traits mode BEFORE including any gcem header. // Without CONSTEIG_GCEM_USE_STDLIB, gcem uses compiler builtins only // (freestanding). With it, gcem uses <limits> and <type_traits>. -#ifndef CONSTEIG_GCEM_USE_STDLIB +#if !CONSTEIG_GCEM_USE_STDLIB `#define` GCEM_TRAITS_BUILTIN `#endif`And in
include/consteig/consteig_options.hpp, provide numeric defaults so undefined →0:+#ifndef CONSTEIG_USE_GCEM +#define CONSTEIG_USE_GCEM 0 +#endif +#ifndef CONSTEIG_GCEM_USE_STDLIB +#define CONSTEIG_GCEM_USE_STDLIB 0 +#endif
61-76: The scalarexp/cos/sinresolve correctly in both backends. In the GCEM path, they are brought in viausingdeclarations (lines 23-30) before the template definitions. In the non-GCEM path,functions/exp.hpp,functions/trig.hpp, andfunctions/sqrt.hppeach declare these functions within theconsteignamespace (confirmed in lines 8-10 and 9-10 of those headers respectively). Whenconstmath.hppreopens theconsteignamespace at line 51 to define theabs(Complex<T>)andexp(Complex<T>)templates, unqualified lookups toexp,cos,sin, andsqrtcorrectly resolve through namespace scope.include/consteig/math/complex.hpp (1)
4-6: LGTM — extractingabs(Complex<T>)resolves the earlier name-lookup concern.With
abs(const Complex<T>&)moved toconstmath.hpp(wheresqrtis guaranteed to be in scope — either viafunctions/sqrt.hpporusing gcem::sqrt),complex.hppno longer needsfunctions/sqrt.hppand is consistently self-sufficient for the symbols it still defines. Good refactor.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/consteig/math/constmath.hpp`:
- Around line 18-33: The abs(const Complex<T>&) implementation should be
declared noexcept to match exp(const Complex<T>&) so noexcept-propagation and
SFINAE behave consistently; update the declaration/definition of
template<typename T> constexpr T abs(const Complex<T>& c) to be noexcept
(preserving constexpr and the current implementation using sqrt) so both abs and
exp have the same exception specification.
In `@include/consteig/math/math_backend.hpp`:
- Around line 65-68: The equalWithin<T, U> implementation uses
static_cast<T>(thresh) which can silently narrow a wider U (e.g., double thresh
vs float T); update the docstring near the equalWithin declaration (around the
existing comment block) to explicitly state that thresh is cast to T and may be
narrowed, and advise callers to pre-cast/choose matching types or use a
common_type-like approach if precision matters; reference the equalWithin
template so maintainers know which function the note applies to.
- Around line 7-46: The header defines GCEM_TRAITS_BUILTIN before including
"../optional_dependencies/gcem/gcem.hpp" but never undefines it, causing the
macro to leak; update math_backend.hpp so that GCEM_TRAITS_BUILTIN is undefined
immediately after the gcem include (while keeping the CONSTEIG_USE_GCEM guard
and existing includes intact) to ensure the trait-mode macro does not affect
other translation units or later includes of gcem.
In `@scripts/vendor_gcem.sh`:
- Around line 38-44: The script currently copies headers from "$TMPDIR_CLONE" to
"$REPO_ROOT/$DEST" but omits upstream license metadata; update
scripts/vendor_gcem.sh to also detect and copy the upstream LICENSE (and if
present NOTICE and AUTHORS) from "$TMPDIR_CLONE" into "$REPO_ROOT/$DEST"
alongside gcem.hpp and gcem_incl using the same cp logic (use existence checks
for LICENSE, NOTICE, AUTHORS and copy each to "$REPO_ROOT/$DEST"), and update
the echo summary if desired; reference the existing variables TMPDIR_CLONE,
REPO_ROOT, DEST and the cp lines that handle gcem.hpp and gcem_incl to locate
where to add the additional copy steps.
- Line 13: The script currently defaults REF="${2:-develop}", which is fragile;
either change the default to a pinned tag (e.g., REF="${2:-v2.0.0}") so re-runs
are deterministic, or keep the parameter but after cloning/resolving the repo
(in the block that uses REF and runs git clone/checkout), capture the exact
commit SHA and write it to a VENDORED_FROM file (e.g., use git -C <repo-dir>
rev-parse --verify HEAD > VENDORED_FROM) alongside the vendored headers so the
vendored snapshot is self-describing; update any README/comments to note the
VENDORED_FROM file if you add it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2fa497f5-62b0-4482-ad36-ddb6a84ea3f5
📒 Files selected for processing (7)
include/consteig/math/constmath.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/utilities.hppinclude/consteig/math/math_backend.hppinclude/consteig/matrix/matrix.hppscripts/vendor_gcem.sh
💤 Files with no reviewable changes (1)
- include/consteig/math/functions/utilities.hpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: gcc-test
- GitHub Check: clang-test-gcem-stdlib
- GitHub Check: clang-test-gcem
- GitHub Check: gcc-test-gcem
- GitHub Check: gcc-test-gcem-stdlib
- GitHub Check: clang-test
- GitHub Check: cross-arm-gcc-gcem
- GitHub Check: cross-arm-clang
- GitHub Check: cross-arm-clang-gcem
- GitHub Check: cross-arm-gcc
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (AGENT.md)
Always use braces for all control flow bodies (if, else, for, while), even single-line ones. No braceless statements.
Files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
include/consteig/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
**/*.hpp
⚙️ CodeRabbit configuration file
**/*.hpp: This is a constexpr C++17 header-only library. Focus on:
- Correctness of constexpr computations and static_assert usage
- Type consistency (Matrix<float,...> must use float tolerances, Matrix<double,...> must use double)
- Explicit casts when indexing Eigen matrices with consteig::Size (unsigned → signed narrowing)
- Tolerance constants are defined in test_dependencies/test_tools.hpp — never hardcode tolerance values, and use the constant appropriate for the method type (direct, iterative, pathological, large magnitude)
- No standard library dependencies in core library code (freestanding constraint)
Files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
🧠 Learnings (31)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:25.802Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:25.802Z
Learning: In production headers under the math/, matrix/, or eigen/ directories, do not introduce or include standard library headers. This guideline prevents std dependency in these header-only/zero-dependency components. If you need NaN or similar constants, use compiler built-ins that are constexpr-compatible (e.g., __builtin_nan(""), __builtin_nanf(""), __builtin_nanl("")); note that the empty string argument is required for constexpr support, and non-empty strings like "nan" are not constant expressions. Apply this guidance to any production headers in these directories; tests may diverge and still use standard headers there.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-25T07:42:50.869Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 46
File: matrix/tests/matrix.test.cpp:442-445
Timestamp: 2026-03-25T07:42:50.869Z
Learning: In `matrix/tests/matrix.test.cpp` (MitchellThompkins/consteig), use `ASSERT_EQ` (not `EXPECT_NEAR`) for runtime checks in `make_matrix` tests. The function merely stores provided literal values into the matrix with no floating-point arithmetic, so the retrieved values are bit-for-bit identical to the inputs — exact equality is correct and `EXPECT_NEAR` with a tolerance constant is not needed.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE for double matrices in tests of iterative methods (Hessenberg, QR iteration).
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-03-28T08:54:06.975Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: docs/guide/matrix.md:133-135
Timestamp: 2026-03-28T08:54:06.975Z
Learning: In MitchellThompkins/consteig (`matrix/matrix.hpp`), the no-arg `isSymmetric()` overload uses `epsilon<T>()` (machine epsilon) as its tolerance — NOT `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE`. `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE` is the eigensolver's routing threshold, passed explicitly as an argument to the `isSymmetric(thresh)` overload inside `eig()` in `eigen/eigen.hpp`. Do not conflate the two.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to include/consteig/**/*.{cpp,hpp} : All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_ITERATIVE_FLOAT_TOLERANCE for float matrices in tests of iterative methods (Hessenberg, QR iteration) to account for error accumulation.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:52:20.667Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Applied to files:
include/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
include/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T05:08:21.606Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:21.606Z
Learning: In math/functions/utilities.hpp of the consteig project, the NaN generation via static_cast<T>(0.0) / static_cast<T>(0.0) is an intentional design choice: it yields NaN at runtime on IEEE 754 platforms, but triggers a compile-time error (UB in constant expressions) in constexpr evaluation. Treat this as acceptable for a compile-time-first library: attempting to compute sqrt(negative) at compile time should be an error, and a separate constexpr path (csqrt()) should be used for negative inputs in constexpr contexts. There is no portable, std-free, ifdef-free, constexpr-NaN that works across GCC, Clang, and MSVC; this approach is the best given the project constraints. When reviewing, preserve this rationale and ensure any refactoring maintaining the same dual behavior, and document constraints about compiler support if modifying NaN generation mechanisms.
Applied to files:
include/consteig/math/math_backend.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-04-23T06:30:20.825Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Applied to files:
scripts/vendor_gcem.shinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:30:36.174Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:8-25
Timestamp: 2026-03-29T19:30:36.174Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `_consteig_version` fallback to `"0.0.0"` when Git or a tag is unavailable (shallow clones, source tarballs) is intentional. Do not suggest adding a dedicated `CONSTEIG_VERSION` cache/env override variable — packagers who need an explicit version can pass `-D_consteig_version=X.Y.Z` at configure time. This is a deliberate design choice, not an oversight.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-19T02:27:16.154Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Dockerfile:31-31
Timestamp: 2026-03-19T02:27:16.154Z
Learning: In MitchellThompkins/consteig (Dockerfile), `cd /tmp` before `sha256sum -c ${ARM_GNU_TOOLCHAIN_FILE}.sha256asc` is intentional and required. BusyBox's `sha256sum -c` resolves filenames inside the checksum file relative to CWD, not relative to the checksum file's location. The ARM `.sha256asc` file contains a bare filename (no path prefix), so `cd /tmp` is needed to make the bare filename resolve correctly. Do not suggest replacing `cd /tmp` with an absolute path to the checksum file, as that does not fix the resolution issue.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T05:47:35.031Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: Makefile:4-4
Timestamp: 2026-03-23T05:47:35.031Z
Learning: In MitchellThompkins/consteig, the `.env` file is tracked in the repository itself. It is always present and always defines `CONTAINER_TAG`. Do not flag the `CONTAINER_TAG := $(shell grep ^CONTAINER_TAG .env | cut -d= -f2)` line in the Makefile as potentially producing an empty value — the `.env` file is guaranteed to exist.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T06:44:11.621Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: .github/workflows/main.yml:24-46
Timestamp: 2026-03-23T06:44:11.621Z
Learning: In MitchellThompkins/consteig, direct pushes to `develop` and `main` branches are blocked by repository branch protection settings. All changes must go through pull requests. Therefore, hardcoding `dockerfile_changed=false` and `env_changed=false` in the `else` branch of the detect-changes step in `.github/workflows/main.yml` is intentional and safe — the guard only needs to run for PR events.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
scripts/vendor_gcem.shinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:58:46.252Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Applied to files:
include/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T07:47:11.893Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/sqrt.hpp:0-0
Timestamp: 2026-03-14T07:47:11.893Z
Learning: In math/functions/utilities.hpp (consteig project), the final design for poison_nan<T>() and force_compile_time_error_for_negative_sqrt<T>() distinguishes between types: for floating-point types, the poison value is static_cast<T>(0.0) / static_cast<T>(0.0) (true IEEE 754 NaN), and is_poison_nan uses x != x; for integer types, the poison value is static_cast<T>(-1) and is_poison_nan checks x == -1. The non-constexpr nature of force_compile_time_error_for_negative_sqrt ensures compile-time evaluation of sqrt(negative) is a hard build error, while runtime returns a meaningful poison value per type. Do not revert this type-discriminated behavior.
Applied to files:
include/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-03-27T19:12:21.039Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: math/functions/pow.hpp:24-36
Timestamp: 2026-03-27T19:12:21.039Z
Learning: In `math/functions/pow.hpp` (MitchellThompkins/consteig), the signed-exponent overload `template <typename T> constexpr T pow(const T x, const int n)` intentionally accepts integral T with positive n (e.g., `pow<int>(x, 3)`). `n` is a runtime parameter so `static_assert(n >= 0)` is not a valid compile-time guard. There is no clean way to restrict negative exponents to floating-point types without changing the API (e.g., using a non-type template parameter). Do not suggest static_assert or if-constexpr guards on runtime function parameters.
Applied to files:
include/consteig/math/functions/csqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use PATHOLOGICAL_TOL for tests involving defective or ill-conditioned matrices to account for theoretical accuracy limitations.
Applied to files:
include/consteig/math/functions/sqrt.hpp
📚 Learning: 2026-04-24T06:22:56.386Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:6-13
Timestamp: 2026-04-24T06:22:56.386Z
Learning: In MitchellThompkins/consteig, `CONSTEIG_USE_GCEM` and `CONSTEIG_GCEM_USE_STDLIB` are presence-based feature-enable macros: they are either defined (feature on) or not defined (feature off). `#ifdef`/`#ifndef` is the correct and idiomatic check for these macros. The `-DMACRO=0` usage pattern is non-standard for this project. Do not flag `#ifdef CONSTEIG_USE_GCEM` or `#ifndef CONSTEIG_GCEM_USE_STDLIB` as fragile or incorrect; they are intentional and correct.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
include/consteig/math/constmath.hpp
📚 Learning: 2026-03-19T02:38:50.626Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: cmake/toolchains/arm-none-eabi-clang.cmake:13-14
Timestamp: 2026-03-19T02:38:50.626Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the library is and will always be a C++ header-only library (driven by its constexpr design goal). There is no C source compilation, so `CMAKE_C_FLAGS_INIT` in toolchain files (e.g., `cmake/toolchains/arm-none-eabi-clang.cmake`) does not need `-isystem` include paths for C++ or GCC headers. Do not suggest adding such paths to `CMAKE_C_FLAGS_INIT` for "completeness" or "future-proofing".
Applied to files:
include/consteig/math/constmath.hpp
🔇 Additional comments (4)
include/consteig/math/functions/sqrt.hpp (1)
5-6: LGTM — explicitabs.hppinclude matches actual usage.
abs(...)is invoked unqualified insidesqrt_recur(line 17) andsqrt_check(line 26), so making the dependency explicit (rather than relying on transitive inclusion throughutilities.hpp) is the right move. No std leaks, all constexpr — librarian-approved. 📚include/consteig/math/functions/csqrt.hpp (1)
4-4: Unqualifiedsqrt/absresolve correctly via the new backend header.Inside
namespace consteig, unqualified lookup picks up eitherconsteig::sqrt/abs(built-in path) or theusing gcem::sqrt; using gcem::abs;re-exports (gcem path) frommath_backend.hpp. All three call sites (lines 28, 36, 41) work for bothis_float<T>and integer branches, so the simplification is sound. Like swapping a hand-cranked record player for a streaming service — same tunes, fewer moving parts. 🎵Also applies to: 28-28, 36-36, 41-41
include/consteig/matrix/matrix.hpp (1)
5-5: Include swap is clean —math_backend.hppre-exports everythingmatrix.hppactually consumes.
epsilon<T>(),is_float<T>()(utilities.hpp, transitively re-included bymath_backend.hppat its line 5), andconsteig::equalWithin(now defined inmath_backend.hppitself at lines 65–68) cover every dependency in this file (lines 319, 322, 352, 365, 391). No transitive surprises, and matrix.hpp is now backend-agnostic for free. 🎯include/consteig/math/math_backend.hpp (1)
24-31: All eightusing gcem::...symbols are present in the vendored gcem library; no compilation issues.The verification confirms that
abs,cos,exp,pow,sgn,sin,sqrt, andtanare all valid exports from gcem. Theusingdeclarations will compile successfully in any TU that includesmath_backend.hppwithCONSTEIG_USE_GCEMdefined.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Owner
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
✅ Created PR with unit tests: #64 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CMakeLists.txt`:
- Around line 80-82: The SYSTEM include for the vendored optional_dependencies
is only added for BUILD_INTERFACE, so installed consumers lose the SYSTEM flag
and get warnings; update the target_include_directories call for target
"consteig" to also emit an INSTALL_INTERFACE entry that mirrors the installed
path (use
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/consteig/optional_dependencies>
alongside the existing $<BUILD_INTERFACE:...> entry) so downstream projects see
the vendored subtree as a SYSTEM include when using the installed package.
In `@include/consteig/math/functions/csqrt.hpp`:
- Around line 39-42: The positive branch in csqrt.hpp returns {sqrt(x),
static_cast<T>(0)} which can cause a narrowing conversion when T is integral
because sqrt(x) is double; mirror the negative branch by explicitly casting the
real part to T (i.e., replace sqrt(x) with static_cast<T>(sqrt(x))) so the
aggregate initializer for Complex<T> performs a narrowing-safe cast; update the
return in the else branch of the csqrt<T> implementation accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0244bf47-0105-4bad-8aaf-2d8be20817de
⛔ Files ignored due to path filters (1)
include/consteig/optional_dependencies/gcem/LICENSEis excluded by!**/optional_dependencies/**
📒 Files selected for processing (11)
CMakeLists.txtdocs/guide/configuration.mddocs/guide/math-functions.mdinclude/consteig/consteig_options.hppinclude/consteig/math/constmath.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/utilities.hppinclude/consteig/math/math_backend.hppinclude/consteig/matrix/matrix.hppscripts/vendor_gcem.sh
💤 Files with no reviewable changes (1)
- include/consteig/math/functions/utilities.hpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (AGENT.md)
Always use braces for all control flow bodies (if, else, for, while), even single-line ones. No braceless statements.
Files:
include/consteig/math/functions/sqrt.hppinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
include/consteig/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Files:
include/consteig/math/functions/sqrt.hppinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
**/*.hpp
⚙️ CodeRabbit configuration file
**/*.hpp: This is a constexpr C++17 header-only library. Focus on:
- Correctness of constexpr computations and static_assert usage
- Type consistency (Matrix<float,...> must use float tolerances, Matrix<double,...> must use double)
- Explicit casts when indexing Eigen matrices with consteig::Size (unsigned → signed narrowing)
- Tolerance constants are defined in test_dependencies/test_tools.hpp — never hardcode tolerance values, and use the constant appropriate for the method type (direct, iterative, pathological, large magnitude)
- No standard library dependencies in core library code (freestanding constraint)
Files:
include/consteig/math/functions/sqrt.hppinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
🧠 Learnings (40)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
📚 Learning: 2026-03-14T04:58:46.252Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppCMakeLists.txtinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppCMakeLists.txtinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:25.802Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:25.802Z
Learning: In production headers under the math/, matrix/, or eigen/ directories, do not introduce or include standard library headers. This guideline prevents std dependency in these header-only/zero-dependency components. If you need NaN or similar constants, use compiler built-ins that are constexpr-compatible (e.g., __builtin_nan(""), __builtin_nanf(""), __builtin_nanl("")); note that the empty string argument is required for constexpr support, and non-empty strings like "nan" are not constant expressions. Apply this guidance to any production headers in these directories; tests may diverge and still use standard headers there.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to include/consteig/**/*.{cpp,hpp} : All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppCMakeLists.txtinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T07:47:11.893Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/sqrt.hpp:0-0
Timestamp: 2026-03-14T07:47:11.893Z
Learning: In math/functions/utilities.hpp (consteig project), the final design for poison_nan<T>() and force_compile_time_error_for_negative_sqrt<T>() distinguishes between types: for floating-point types, the poison value is static_cast<T>(0.0) / static_cast<T>(0.0) (true IEEE 754 NaN), and is_poison_nan uses x != x; for integer types, the poison value is static_cast<T>(-1) and is_poison_nan checks x == -1. The non-constexpr nature of force_compile_time_error_for_negative_sqrt ensures compile-time evaluation of sqrt(negative) is a hard build error, while runtime returns a meaningful poison value per type. Do not revert this type-discriminated behavior.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/csqrt.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:52:20.667Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-28T08:54:06.975Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: docs/guide/matrix.md:133-135
Timestamp: 2026-03-28T08:54:06.975Z
Learning: In MitchellThompkins/consteig (`matrix/matrix.hpp`), the no-arg `isSymmetric()` overload uses `epsilon<T>()` (machine epsilon) as its tolerance — NOT `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE`. `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE` is the eigensolver's routing threshold, passed explicitly as an argument to the `isSymmetric(thresh)` overload inside `eig()` in `eigen/eigen.hpp`. Do not conflate the two.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE for double matrices in tests of iterative methods (Hessenberg, QR iteration).
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_ITERATIVE_FLOAT_TOLERANCE for float matrices in tests of iterative methods (Hessenberg, QR iteration) to account for error accumulation.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
include/consteig/math/functions/sqrt.hppdocs/guide/configuration.mdinclude/consteig/consteig_options.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppCMakeLists.txtdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T05:08:21.606Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:21.606Z
Learning: In math/functions/utilities.hpp of the consteig project, the NaN generation via static_cast<T>(0.0) / static_cast<T>(0.0) is an intentional design choice: it yields NaN at runtime on IEEE 754 platforms, but triggers a compile-time error (UB in constant expressions) in constexpr evaluation. Treat this as acceptable for a compile-time-first library: attempting to compute sqrt(negative) at compile time should be an error, and a separate constexpr path (csqrt()) should be used for negative inputs in constexpr contexts. There is no portable, std-free, ifdef-free, constexpr-NaN that works across GCC, Clang, and MSVC; this approach is the best given the project constraints. When reviewing, preserve this rationale and ensure any refactoring maintaining the same dual behavior, and document constraints about compiler support if modifying NaN generation mechanisms.
Applied to files:
include/consteig/math/functions/sqrt.hppinclude/consteig/math/functions/csqrt.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Applied to files:
docs/guide/configuration.mdinclude/consteig/consteig_options.hppCMakeLists.txtdocs/guide/math-functions.md
📚 Learning: 2026-04-24T06:22:56.386Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:6-13
Timestamp: 2026-04-24T06:22:56.386Z
Learning: In MitchellThompkins/consteig, `CONSTEIG_USE_GCEM` and `CONSTEIG_GCEM_USE_STDLIB` are presence-based feature-enable macros: they are either defined (feature on) or not defined (feature off). `#ifdef`/`#ifndef` is the correct and idiomatic check for these macros. The `-DMACRO=0` usage pattern is non-standard for this project. Do not flag `#ifdef CONSTEIG_USE_GCEM` or `#ifndef CONSTEIG_GCEM_USE_STDLIB` as fragile or incorrect; they are intentional and correct.
Applied to files:
docs/guide/configuration.mdinclude/consteig/consteig_options.hppCMakeLists.txtinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
docs/guide/configuration.mdinclude/consteig/consteig_options.hppCMakeLists.txtinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-04-23T06:30:20.825Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Applied to files:
docs/guide/configuration.mdscripts/vendor_gcem.shinclude/consteig/consteig_options.hppCMakeLists.txtinclude/consteig/math/math_backend.hppdocs/guide/math-functions.mdinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
docs/guide/configuration.mdCMakeLists.txtinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
docs/guide/configuration.mdinclude/consteig/math/functions/csqrt.hppinclude/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T19:30:36.174Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:8-25
Timestamp: 2026-03-29T19:30:36.174Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `_consteig_version` fallback to `"0.0.0"` when Git or a tag is unavailable (shallow clones, source tarballs) is intentional. Do not suggest adding a dedicated `CONSTEIG_VERSION` cache/env override variable — packagers who need an explicit version can pass `-D_consteig_version=X.Y.Z` at configure time. This is a deliberate design choice, not an oversight.
Applied to files:
scripts/vendor_gcem.shCMakeLists.txt
📚 Learning: 2026-03-19T02:27:16.154Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Dockerfile:31-31
Timestamp: 2026-03-19T02:27:16.154Z
Learning: In MitchellThompkins/consteig (Dockerfile), `cd /tmp` before `sha256sum -c ${ARM_GNU_TOOLCHAIN_FILE}.sha256asc` is intentional and required. BusyBox's `sha256sum -c` resolves filenames inside the checksum file relative to CWD, not relative to the checksum file's location. The ARM `.sha256asc` file contains a bare filename (no path prefix), so `cd /tmp` is needed to make the bare filename resolve correctly. Do not suggest replacing `cd /tmp` with an absolute path to the checksum file, as that does not fix the resolution issue.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T05:47:35.031Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: Makefile:4-4
Timestamp: 2026-03-23T05:47:35.031Z
Learning: In MitchellThompkins/consteig, the `.env` file is tracked in the repository itself. It is always present and always defines `CONTAINER_TAG`. Do not flag the `CONTAINER_TAG := $(shell grep ^CONTAINER_TAG .env | cut -d= -f2)` line in the Makefile as potentially producing an empty value — the `.env` file is guaranteed to exist.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T06:44:11.621Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: .github/workflows/main.yml:24-46
Timestamp: 2026-03-23T06:44:11.621Z
Learning: In MitchellThompkins/consteig, direct pushes to `develop` and `main` branches are blocked by repository branch protection settings. All changes must go through pull requests. Therefore, hardcoding `dockerfile_changed=false` and `env_changed=false` in the `else` branch of the detect-changes step in `.github/workflows/main.yml` is intentional and safe — the guard only needs to run for PR events.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
scripts/vendor_gcem.shCMakeLists.txtinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T04:19:13.993Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 57
File: docs/getting-started.md:35-35
Timestamp: 2026-03-30T04:19:13.993Z
Learning: In MitchellThompkins/consteig (`docs/getting-started.md`), the FetchContent `GIT_TAG` in the getting-started example was changed from `main` to the explicit release tag (e.g., `1.0.0`) as of the 1.0.0 release PR (`#57`). This is intentional and consistent with the rest of the docs (Conan, vcpkg all pin to the same version). Do not suggest reverting to `main` — pinning to the current release tag is now the intended policy for this file.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-27T19:12:21.039Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: math/functions/pow.hpp:24-36
Timestamp: 2026-03-27T19:12:21.039Z
Learning: In `math/functions/pow.hpp` (MitchellThompkins/consteig), the signed-exponent overload `template <typename T> constexpr T pow(const T x, const int n)` intentionally accepts integral T with positive n (e.g., `pow<int>(x, 3)`). `n` is a runtime parameter so `static_assert(n >= 0)` is not a valid compile-time guard. There is no clean way to restrict negative exponents to floating-point types without changing the API (e.g., using a non-type template parameter). Do not suggest static_assert or if-constexpr guards on runtime function parameters.
Applied to files:
include/consteig/math/functions/csqrt.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hpp
📚 Learning: 2026-03-25T07:42:50.869Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 46
File: matrix/tests/matrix.test.cpp:442-445
Timestamp: 2026-03-25T07:42:50.869Z
Learning: In `matrix/tests/matrix.test.cpp` (MitchellThompkins/consteig), use `ASSERT_EQ` (not `EXPECT_NEAR`) for runtime checks in `make_matrix` tests. The function merely stores provided literal values into the matrix with no floating-point arithmetic, so the retrieved values are bit-for-bit identical to the inputs — exact equality is correct and `EXPECT_NEAR` with a tolerance constant is not needed.
Applied to files:
include/consteig/matrix/matrix.hppinclude/consteig/math/math_backend.hppinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
CMakeLists.txtinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
CMakeLists.txtinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-16T01:25:54.839Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: CMakeLists.txt:27-30
Timestamp: 2026-03-16T01:25:54.839Z
Learning: In CMakeLists.txt, keep C++ standard settings (CMAKE_CXX_STANDARD and CMAKE_CXX_EXTENSIONS OFF) inside the if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) guard. This ensures the consumer's C++ standard is not overridden when the project is added via add_subdirectory or FetchContent. Do not move these settings outside the guard; if a consumer does not opt into C++17+, their build should fail to compile due to C++17 syntax.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T18:43:47.241Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:0-0
Timestamp: 2026-03-29T18:43:47.241Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `project()` call explicitly declares `LANGUAGES CXX` (e.g., `project(consteig VERSION ${_consteig_version} LANGUAGES CXX)`). Since the library is header-only C++ with no C sources, this avoids an unnecessary C compiler probe at configure time. Do not flag the absence of a C language declaration as an issue.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-19T02:38:50.626Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: cmake/toolchains/arm-none-eabi-clang.cmake:13-14
Timestamp: 2026-03-19T02:38:50.626Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the library is and will always be a C++ header-only library (driven by its constexpr design goal). There is no C source compilation, so `CMAKE_C_FLAGS_INIT` in toolchain files (e.g., `cmake/toolchains/arm-none-eabi-clang.cmake`) does not need `-isystem` include paths for C++ or GCC headers. Do not suggest adding such paths to `CMAKE_C_FLAGS_INIT` for "completeness" or "future-proofing".
Applied to files:
CMakeLists.txtinclude/consteig/math/constmath.hpp
📚 Learning: 2026-03-29T17:49:00.041Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:171-180
Timestamp: 2026-03-29T17:49:00.041Z
Learning: For CMake projects, avoid hardcoding the project version in the `project()` call. Instead, derive the version at configure time from Git tags by running `git describe --tags --abbrev=0`, strip a leading `v` if present (e.g., `v1.2.3` -> `1.2.3`), and pass the resulting value to `project(<name> VERSION <derived_version> ...)`. Do not suggest adding a static `VERSION` argument to `project()` when the repository already intends to source it from Git tags.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Preserve the float vs double type distinction in test matrices. Do not change Matrix<float, ...> to Matrix<double, ...> or vice versa, as tests are intentionally typed for precision validation.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: When resolving merge conflicts between strict-build branches and main, add new tolerance constants to test_dependencies/test_tools.hpp if needed for type consistency.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use explicit static_cast<Eigen::Index>(i) when indexing Eigen matrices with Size variables to satisfy strict build warnings about narrowing conversions.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use LARGE_VAL_TOL when comparing results with large magnitude values.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-03-14T05:08:31.504Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:31.504Z
Learning: In math/functions/utilities.hpp in the consteig project (MitchellThompkins/consteig), the nan<T>() function using `static_cast<T>(0.0) / static_cast<T>(0.0)` is intentional design: at runtime on IEEE 754 platforms it produces NaN, and in a constexpr evaluation context it produces a compile error (UB in constant expressions). This dual behavior is acceptable and even desirable for a compile-time-first library — sqrt(negative) at compile time should be an error, and csqrt() is the correct path for negative inputs in constexpr contexts. There is no portable C++17, std-free, ifdef-free, constexpr NaN that works across GCC, Clang, AND MSVC; the 0.0/0.0 approach is the best available option given the project's constraints.
Applied to files:
docs/guide/math-functions.md
🔇 Additional comments (11)
scripts/vendor_gcem.sh (2)
13-13:REF=developdefault — reproducibility nit (already noted, leaving here for visibility).Still recommending pinning to a tag or persisting the resolved SHA into a
VENDORED_FROMsidecar so re-runs are deterministic. No new analysis beyond the prior round.
35-41: Past round of feedback landed nicely.
${REPO_ROOT:?}/${DEST:?}(line 35) defangs the rm, and the newLICENSEcopy at line 41 satisfies the upstream Apache-2.0 attribution requirement. Re-running with a hard-fail if upstream ever renames the file is acceptable behaviour for a vendoring tool.include/consteig/math/constmath.hpp (1)
18-33: Complexabs/expoverloads land cleanly;noexceptasymmetry between the two is unchanged from prior round.Overload resolution is unambiguous (
Complex<T>&vs scalarT), the scalarsqrt/exp/cos/sincalls correctly route throughmath_backend.hppunder either backend, and both functions stayconstexpr/freestanding per the include/consteig contract. Thenoexcept-on-exp-but-not-absmismatch at lines 18 vs 29 was already raised — not re-raising.include/consteig/math/functions/sqrt.hpp (1)
5-6: LGTM — explicitabs.hppinclude is the right move.
abs(...)is used unqualified at lines 17 and 26, so adding the explicit include makes the dependency self-evident and avoids relying on transitive includes fromutilities.hpp. Aligns nicely with the math_backend split where this header is only consumed in the non-GCEM path.include/consteig/consteig_options.hpp (1)
85-137: Documentation block reads cleanly and matches the wiring inmath_backend.hpp.The three commented-out option macros are well-explained, the ODR warning on
CONSTEIG_GCEM_USE_CUSTOM_TRAITSis appropriately loud, and the mutual-exclusivity note on lines 110 and 136 matches the#ifdef …#elifdefined(...)chain ininclude/consteig/math/math_backend.hpp. Presence-based#ifdef/#ifndefis the intended idiom here.Based on learnings: "In MitchellThompkins/consteig,
CONSTEIG_USE_GCEMandCONSTEIG_GCEM_USE_STDLIBare presence-based feature-enable macros …#ifdef/#ifndefis the correct and idiomatic check for these macros."include/consteig/matrix/matrix.hpp (1)
5-5: Header swap is correct.
math_backend.hppre-exportsequalWithin,epsilon,is_float, andabs(either consteig built-ins orusing gcem::abs) intonamespace consteig, which is exactly what theisSymmetric()overloads at lines 321 and 365 andequalWithinat line 391 need. Nostdheaders introduced — freestanding contract preserved.include/consteig/math/functions/csqrt.hpp (1)
28-37: Negative branches look good post-refactor.Float path at line 28 is gated by
is_float<T>()sosqrt(abs(x))returnsTunder both backends. Integer path at lines 35–36 keeps the explicitstatic_cast<T>and theunsigned long longtrick to handleINT_MIN/INT64_MINwithout UB. Behaviour is preserved across backends here.docs/guide/configuration.md (1)
53-82: New gcem option sections are accurate and well-cross-linked.The link target
math-functions.md#optional-gcem-backend(line 56) andmath-functions.md#custom-traits(line 80) both resolve against the matching headings indocs/guide/math-functions.md. Mutual-exclusivity note on line 67–68 and 81 is consistent with the implementation ininclude/consteig/math/math_backend.hppand the macro docs ininclude/consteig/consteig_options.hpp.docs/guide/math-functions.md (1)
84-138: "Optional gcem Backend" section is accurate and matches the implementation.The traits-mode table on lines 99–103, the ODR warning on lines 95–97, and the mutual-exclusivity statement on line 105 are all consistent with the
#ifdef CONSTEIG_GCEM_USE_STDLIB/#elif defined(CONSTEIG_GCEM_USE_CUSTOM_TRAITS)chain ininclude/consteig/math/math_backend.hpp. The custom-traits example at lines 112–124 correctly orders thenamespace gcemdecls before the#defineand the consteig include — that's the only ordering that works sinceconsteig_options.hppdoes not predefine the macro.include/consteig/math/math_backend.hpp (2)
7-46: Trait-mode dispatch + scoped#undefcleanup look right.Three-way dispatch matches the documented contract (stdlib → no macro / gcem default hosted, custom →
GCEM_TRAITS_CUSTOM, else →GCEM_TRAITS_BUILTIN), and the post-include#undef GCEM_TRAITS_BUILTIN/#undef GCEM_TRAITS_CUSTOMat lines 25–26 closes the prior macro-leak concern cleanly. Mutual exclusion of the two trait modes is enforced at configure time inCMakeLists.txt(line 69–71), so the#elifchain here doesn't need to re-check.
5-5: No issues to address.functions/utilities.hppis self-contained (no transitive includes) and defines only utility helpers likeis_float(),epsilon(), andpoison_nan()—not the per-function math symbols (abs,sqrt,exp, etc.). Theusing gcem::*re-exports in the gcem branch have no collision risk.
Owner
Author
@coderabbitai why did you make this PR? I didn't ask you to. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/vendor_gcem.sh (1)
7-13: 🧹 Nitpick | 🔵 TrivialTiny doc/default mismatch — usage example advertises a newer tag than the default.
The usage example on line 8 hands
v.1.19.0-consteig.2to the script while the defaultREFon line 13 isv.1.19.0-consteig.1. Harmless (the example is just showing "how to bump"), but a future reader may briefly wonder which is the "current" pinned ref. Consider keeping the example tag in sync with the default, or rewording the comment to make it explicit that the example shows bumping to a newer tag.✏️ Optional polish
-# To update to a new branch or tag, pass different arguments: -# ./scripts/vendor_gcem.sh https://github.com/MitchellThompkins/gcem.git v.1.19.0-consteig.2 +# To update to a new branch or tag, pass different arguments. For example, to +# bump from the current default to a newer tag: +# ./scripts/vendor_gcem.sh https://github.com/MitchellThompkins/gcem.git v.1.19.0-consteig.2Side note:
git clone --depth=1 --branch "$REF"only accepts a branch or tag name — a commit SHA inREFwill fail with a non-obvious error. Was raised previously alongside the SC2115 fix; flagging here only as a reminder in case you want a one-line inline note next to line 32.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/vendor_gcem.sh` around lines 7 - 13, The usage example in the script header shows tag v.1.19.0-consteig.2 while the default REF is v.1.19.0-consteig.1; update the comment so they match or explicitly state the example is demonstrating how to bump to a newer tag—edit the header comment around the REPO_URL/REF example to either change the example tag to v.1.19.0-consteig.1 or add a short phrase like "example: bumping to v.1.19.0-consteig.2" to make intent clear, and optionally add a one-line note near where REF is used (e.g., around the git clone call) reminding that git clone --branch only accepts branch or tag names, not commit SHAs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/vendor_gcem.sh`:
- Around line 7-13: The usage example in the script header shows tag
v.1.19.0-consteig.2 while the default REF is v.1.19.0-consteig.1; update the
comment so they match or explicitly state the example is demonstrating how to
bump to a newer tag—edit the header comment around the REPO_URL/REF example to
either change the example tag to v.1.19.0-consteig.1 or add a short phrase like
"example: bumping to v.1.19.0-consteig.2" to make intent clear, and optionally
add a one-line note near where REF is used (e.g., around the git clone call)
reminding that git clone --branch only accepts branch or tag names, not commit
SHAs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa816019-a160-404e-a80f-cf371798ada9
📒 Files selected for processing (4)
README.mddocs/getting-started.mddocs/index.mdscripts/vendor_gcem.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: gcc-test-gcem-stdlib
- GitHub Check: clang-test
- GitHub Check: clang-test-gcem-stdlib
- GitHub Check: gcc-test
- GitHub Check: clang-test-gcem
- GitHub Check: cross-arm-clang-gcem
- GitHub Check: gcc-test-gcem
- GitHub Check: cross-arm-gcc-gcem
- GitHub Check: cross-arm-gcc
- GitHub Check: cross-arm-clang
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: scripts/vendor_gcem.sh:13-13
Timestamp: 2026-04-26T06:18:53.922Z
Learning: In MitchellThompkins/consteig (`scripts/vendor_gcem.sh`), the default `REF=develop` is intentional. The upstream gcem repository (https://github.com/MitchellThompkins/gcem) has not yet cut a stable release tag, so `develop` is the correct and deliberate tracking ref for vendoring. Do not flag this default as a reproducibility concern.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
docs/index.mdREADME.md
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Applied to files:
README.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
README.md
📚 Learning: 2026-03-28T08:54:06.975Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: docs/guide/matrix.md:133-135
Timestamp: 2026-03-28T08:54:06.975Z
Learning: In MitchellThompkins/consteig (`matrix/matrix.hpp`), the no-arg `isSymmetric()` overload uses `epsilon<T>()` (machine epsilon) as its tolerance — NOT `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE`. `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE` is the eigensolver's routing threshold, passed explicitly as an argument to the `isSymmetric(thresh)` overload inside `eig()` in `eigen/eigen.hpp`. Do not conflate the two.
Applied to files:
README.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Matrix decompositions use Givens rotations for numerical stability.
Applied to files:
README.md
📚 Learning: 2026-04-23T06:30:20.825Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-04-26T06:18:53.922Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: scripts/vendor_gcem.sh:13-13
Timestamp: 2026-04-26T06:18:53.922Z
Learning: In MitchellThompkins/consteig (`scripts/vendor_gcem.sh`), the default `REF=develop` is intentional. The upstream gcem repository (https://github.com/MitchellThompkins/gcem) has not yet cut a stable release tag, so `develop` is the correct and deliberate tracking ref for vendoring. Do not flag this default as a reproducibility concern.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-29T19:30:36.174Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:8-25
Timestamp: 2026-03-29T19:30:36.174Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `_consteig_version` fallback to `"0.0.0"` when Git or a tag is unavailable (shallow clones, source tarballs) is intentional. Do not suggest adding a dedicated `CONSTEIG_VERSION` cache/env override variable — packagers who need an explicit version can pass `-D_consteig_version=X.Y.Z` at configure time. This is a deliberate design choice, not an oversight.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T06:44:11.621Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: .github/workflows/main.yml:24-46
Timestamp: 2026-03-23T06:44:11.621Z
Learning: In MitchellThompkins/consteig, direct pushes to `develop` and `main` branches are blocked by repository branch protection settings. All changes must go through pull requests. Therefore, hardcoding `dockerfile_changed=false` and `env_changed=false` in the `else` branch of the detect-changes step in `.github/workflows/main.yml` is intentional and safe — the guard only needs to run for PR events.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-23T05:47:35.031Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: Makefile:4-4
Timestamp: 2026-03-23T05:47:35.031Z
Learning: In MitchellThompkins/consteig, the `.env` file is tracked in the repository itself. It is always present and always defines `CONTAINER_TAG`. Do not flag the `CONTAINER_TAG := $(shell grep ^CONTAINER_TAG .env | cut -d= -f2)` line in the Makefile as potentially producing an empty value — the `.env` file is guaranteed to exist.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-19T02:27:16.154Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Dockerfile:31-31
Timestamp: 2026-03-19T02:27:16.154Z
Learning: In MitchellThompkins/consteig (Dockerfile), `cd /tmp` before `sha256sum -c ${ARM_GNU_TOOLCHAIN_FILE}.sha256asc` is intentional and required. BusyBox's `sha256sum -c` resolves filenames inside the checksum file relative to CWD, not relative to the checksum file's location. The ARM `.sha256asc` file contains a bare filename (no path prefix), so `cd /tmp` is needed to make the bare filename resolve correctly. Do not suggest replacing `cd /tmp` with an absolute path to the checksum file, as that does not fix the resolution issue.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-30T04:19:13.993Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 57
File: docs/getting-started.md:35-35
Timestamp: 2026-03-30T04:19:13.993Z
Learning: In MitchellThompkins/consteig (`docs/getting-started.md`), the FetchContent `GIT_TAG` in the getting-started example was changed from `main` to the explicit release tag (e.g., `1.0.0`) as of the 1.0.0 release PR (`#57`). This is intentional and consistent with the rest of the docs (Conan, vcpkg all pin to the same version). Do not suggest reverting to `main` — pinning to the current release tag is now the intended policy for this file.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-29T17:49:06.566Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:171-180
Timestamp: 2026-03-29T17:49:06.566Z
Learning: In MitchellThompkins/consteig, version information is intentionally NOT hardcoded in CMakeLists.txt. The project version should be derived at configure time from Git tags via `git describe --tags --abbrev=0` (stripping a leading "v" if present) and passed to the `project()` call. Do not suggest adding a static VERSION to the `project()` declaration.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-15T20:21:45.214Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: .github/workflows/main.yml:31-31
Timestamp: 2026-03-15T20:21:45.214Z
Learning: In the consteig repository (.github/workflows/main.yml), the `build_and_publish_container` job is intentionally gated to only run on `develop`/`main` branch pushes. All downstream jobs (lint, gcc-test, gcc-examples, clang-test, clang-examples, dc-motor-fail) use `if: always() && (needs.build_and_publish_container.result == 'success' || needs.build_and_publish_container.result == 'skipped')` — the `skipped` branch is required so that PR jobs can still run against the most recently published `:latest` container image. This is a deliberate tradeoff to avoid rebuilding the container on every PR. Do not flag this pattern as a bug.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-19T02:08:07.404Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Dockerfile:27-27
Timestamp: 2026-03-19T02:08:07.404Z
Learning: In MitchellThompkins/consteig (Dockerfile), package versions in `apk add` commands are intentionally left unpinned because this is a development image. The maintainer accepts that rebuilds will pull in newer package versions, and relies on previously published container images as a rollback mechanism if a new dependency breaks things. Do not flag unpinned `apk add` packages as a bug or suggest pinning.
Applied to files:
scripts/vendor_gcem.sh
📚 Learning: 2026-03-16T01:24:22.804Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: Makefile:196-197
Timestamp: 2026-03-16T01:24:22.804Z
Learning: In MitchellThompkins/consteig (Makefile), the `container.make.%` recipe intentionally always forwards `CC=$(CC)` and `CXX=$(CXX)` to the inner `make` call inside Docker, even when those aren't explicitly set by the caller. The GNU Make defaults (`cc`, `g++`) are equivalent to the Alpine container's defaults, so this is intentional and harmless. Do not flag this as a bug.
Applied to files:
scripts/vendor_gcem.sh
🔇 Additional comments (4)
scripts/vendor_gcem.sh (1)
10-45: LGTM — vendoring script is tight and the previously-raised concerns are all addressed.A quick recap of what I re-checked on this pass:
- Line 10:
set -eis in place; the script aborts on any failedgit/cp/mkdir/findstep, so a missing upstreamLICENSEorinclude/gcem.hppwon't silently produce a half-vendored tree. 👍- Line 13: default
REFis now a forked-and-tagged ref (v.1.19.0-consteig.1) per the latest commit, which is strictly better than tracking a moving branch — re-runs are now reproducible by default.- Lines 18–23:
mktemp -d+trap cleanup EXITcorrectly cleans up the clone on both success and failure paths.- Line 32:
--quiet --depth=1 --branch "$REF"is the right shape for a vendoring tool — fast, tag-pinned, and won't spam the terminal with detached-HEAD chatter.- Line 35:
rm -rf "${REPO_ROOT:?}/${DEST:?}"keeps the SC2115-style guard from the prior review round, so an unset/emptyDESTorREPO_ROOTwill abort instead of nuking the repo root. 🔒- Line 41: upstream
LICENSEis now copied alongside the headers, so Apache-2.0 redistribution terms are satisfied and downstream license scanners (FOSSA/Tern/etc.) will be happy.- Line 43:
find … -name "*.hpp" | wc -l | tr -d ' 'is a nice human-readable summary at the end.Nothing blocking from me here. ⚖️
docs/getting-started.md (1)
207-207: LGTM! Link path correction.The change from
.mdfile reference to directory-style URL with trailing slash aligns with common static site generator conventions.README.md (1)
26-26: LGTM! Good documentation consistency.The feature addition matches
docs/index.mdline 29 word-for-word, maintaining consistency across documentation files. The external repository link verification requested fordocs/index.mdapplies here as well.docs/index.md (1)
29-29: The gcem repository link is accessible and points to the correct fork.
This was referenced Apr 26, 2026
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
1 participant
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.
Summary
Adds gcem as an optional compile-time math backend. When enabled,
consteig::math names remain identical; only the underlying implementation changes.What changed
Math backend architecture
A new
math_backend.hppserves as the single backend switch. It either pulls in gcem viausingdeclarations or includes consteig's own built-in implementations. All other headers includemath_backend.hppdirectly, making each header standalone and independent of include ordering.gcem vendoring
gcem headers are vendored under
include/consteig/optional_dependencies/gcem/viascripts/vendor_gcem.sh. The Apache-2.0 LICENSE is copied alongside the headers. The vendored tree is excluded from formatting and code review scope.Configuration options
Three new macros control the gcem trait dispatch mode (see
consteig_options.hppanddocs/guide/configuration.md):CONSTEIG_USE_GCEM-- enables the gcem backendCONSTEIG_GCEM_USE_STDLIB-- switches gcem to use hosted<limits>/<type_traits>instead of compiler builtinsCONSTEIG_GCEM_USE_CUSTOM_TRAITS-- opt-in for supplying your own trait definitions innamespace gcemThe default (no extra macros) uses gcem's freestanding builtin traits, keeping the library compatible with freestanding environments.
CI
The CI matrix is expanded to test all backend variants: built-in, gcem with builtin traits, and gcem with stdlib traits, across GCC, Clang, and cross-compiled ARM targets.
Summary by CodeRabbit
New Features
Chores
Documentation