Skip to content

feat(concepts): add C++20 concepts for operators, metrics, and policy types - #67

Merged
foolnotion merged 10 commits into
mainfrom
feature/concepts
Jun 4, 2026
Merged

feat(concepts): add C++20 concepts for operators, metrics, and policy types#67
foolnotion merged 10 commits into
mainfrom
feature/concepts

Conversation

@foolnotion

@foolnotion foolnotion commented Jun 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds Creator, Mutator, Crossover, Selector, Reinserter, Evaluator, ErrorMetric, and Hasher concepts to include/operon/core/concepts.hpp alongside the existing Arithmetic one. Tree and Individual are forward-declared to keep the header lightweight; Likelihood and OptimizerLoss remain in likelihood_base.hpp where their Eigen/vstat dependencies live.
  • Replaces static_assert + <type_traits> guards in all six error metric free-function headers with requires clauses using Concepts::Arithmetic and std::same_as. Span overloads use the concept directly as the named template parameter.
  • Constrains the Lik template parameter of MinimumDescriptionLengthEvaluator and FractionalBayesFactorEvaluator with Concepts::Likelihood.

Evaluator<DTable> is intentionally left with the runtime ErrorMetric polymorphic member — the CLI relies on dynamic_cast to a single concrete type, so templatising on the metric type would break that.

Test plan

  • Full build passes (cmake --build build -j$(nproc)) with no new errors or warnings
  • Verify static_assert messages are gone and replaced by constraint failures for bad instantiations
  • Check that existing evaluator/error metric tests still pass

foolnotion added 2 commits June 3, 2026 10:25
@foolnotion
feat(concepts): add operator and policy concepts to concepts.hpp
a406611 
Adds Creator, Mutator, Crossover, Selector, Reinserter, Evaluator,
ErrorMetric, and Hasher concepts alongside the existing Arithmetic one.
Tree and Individual are forward-declared to keep the header lightweight.
Likelihood and OptimizerLoss remain in likelihood_base.hpp (Eigen/vstat deps).
@foolnotion
feat(concepts): apply concept constraints to error metrics and evalua…
dfc2fdd 
…tors

Replace static_assert+type_traits guards in all error metric free functions
with requires clauses using Concepts::Arithmetic and std::same_as. Use
Concepts::Arithmetic as a named template parameter on span overloads.

Constrain the Lik template parameter of MinimumDescriptionLengthEvaluator
and FractionalBayesFactorEvaluator with Concepts::Likelihood.
Copilot AI review requested due to automatic review settings June 3, 2026 08:11
Copilot AI reviewed Jun 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes Operon’s C++ type constraints by introducing a set of C++20 concepts in core/concepts.hpp, then applying them to operator/evaluator templates and the error-metric free-function APIs to replace static_assert-based checks with requires clauses.

Changes:

  • Added multiple C++20 concepts (e.g., Creator, Mutator, Crossover, Selector, Reinserter, Evaluator, ErrorMetric, Hasher) to include/operon/core/concepts.hpp.
  • Replaced static_assert + <type_traits> checks in several error metric headers with requires constraints using Concepts::Arithmetic and std::same_as.
  • Constrained the Lik template parameter for MinimumDescriptionLengthEvaluator and FractionalBayesFactorEvaluator to Concepts::Likelihood.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
include/operon/operators/evaluator.hpp Constrains evaluator likelihood template parameters via Concepts::Likelihood.
include/operon/error_metrics/sum_of_squared_errors.hpp Replaces static_assert type guards with requires constraints and concept-bound span overloads.
include/operon/error_metrics/r2_score.hpp Adds requires constraints and refactors type usage in accumulate calls.
include/operon/error_metrics/normalized_mean_squared_error.hpp Adds requires constraints and updates span overloads to use pointer-based calls.
include/operon/error_metrics/mean_squared_error.hpp Adds requires constraints and concept-bound span overloads.
include/operon/error_metrics/mean_absolute_error.hpp Adds requires constraints and concept-bound span overloads.
include/operon/error_metrics/correlation_coefficient.hpp Adds requires constraints and concept-bound span overloads (plus squared correlation helpers).
include/operon/core/concepts.hpp Introduces the new suite of C++20 concepts and forward declarations to reduce heavy includes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/operon/error_metrics/r2_score.hpp Outdated
auto sqres = [](auto a, auto b) { auto e=a-b; return e*e; };
auto const ssr = accumulate<V1>(begin1, end1, begin2, sqres).sum;
auto const sst = accumulate<V2>(begin2, begin2 + std::distance(begin1, end1)).ssr;
auto const sst = accumulate<V1>(begin2, begin2 + std::distance(begin1, end1)).ssr;
Comment thread include/operon/error_metrics/r2_score.hpp Outdated
using vstat::univariate::accumulate;
auto sqres = [](auto a, auto b) { auto e=a-b; return e*e; };
auto const ssr = accumulate<V1>(begin1, end1, begin2, begin3, sqres).sum;
auto end2 = begin2 + std::distance(begin1, end1);
Comment thread include/operon/error_metrics/normalized_mean_squared_error.hpp
static_assert(std::is_arithmetic_v<V1>, "InputIt1: value_type must be arithmetic.");
static_assert(std::is_arithmetic_v<V2>, "InputIt2: value_type must be arithmetic.");
static_assert(std::is_same_v<V1, V2>, "The types must be the same");
auto varY = vstat::univariate::accumulate<V1>(begin2, begin2 + std::distance(begin1, end1)).variance;
Comment thread include/operon/error_metrics/normalized_mean_squared_error.hpp
static_assert(std::is_arithmetic_v<V1>, "InputIt1: value_type must be arithmetic.");
static_assert(std::is_arithmetic_v<V2>, "InputIt2: value_type must be arithmetic.");
static_assert(std::is_same_v<V1, V2>, "The types must be the same");
auto varY = vstat::univariate::accumulate<V1>(begin2, begin2 + std::distance(begin1, end1), begin3).variance;
foolnotion added 6 commits June 3, 2026 11:34
@foolnotion
fix(error_metrics): use std::next instead of iterator + distance
a4c0419 
operator+ on non-random-access iterators is ill-formed; std::next works
for all iterator categories and makes the template requirements explicit.
@foolnotion
fix(error_metrics): constrain iterator overloads to std::random_acces…
0e85c13 
…s_iterator

vstat requires random-access iterators at its interface. Make that
requirement explicit on all metric iterator overloads so that
iterator arithmetic (begin + distance) is well-formed by contract
rather than relying on std::next to paper over the constraint.
@foolnotion
perf(error_metrics): use vstat SIMD-optimized metric functions
142ec34 
MSE, MAE, R2Score, and NMSE iterator overloads now delegate to
vstat::metrics::{mean_squared_error,mean_absolute_error,r2_score}
which use EVE SIMD wide types. Iterator constraints tightened from
std::random_access_iterator to std::contiguous_iterator to match
vstat's interface; raw pointer (Span::data()) callers are unaffected.

SSE and CorrelationCoefficient have no dedicated SIMD path in vstat
and remain on the generic accumulate path (random_access_iterator).
@foolnotion
fix(r2_score): revert to accumulate formula; keep contiguous_iterator…
072bfce 
… constraint

vstat::metrics::r2_score uses a different weighted-variance normalization
that diverges from the Elki reference by ~3e-4 in the weighted case.
Use vstat::univariate::accumulate directly to preserve the correct formula,
while keeping std::contiguous_iterator so the constraint reflects vstat's
actual requirement and begin + distance arithmetic remains valid.
@foolnotion
fix(r2_score): correct comment on SIMD path avoidance
122937f
@foolnotion
fix(r2_score): use vstat SIMD path; fix weighted R2 reference in test
40d02c8 
The test's Elki::MeanVariance::R2(x,y,z) was computing TSS using the
unweighted mean of y instead of the weighted mean, giving a wrong
reference value. Fixed by passing weights to PopulationStats.

With the correct reference, vstat::metrics::r2_score passes at 1e-6
for both the unweighted and weighted cases, so the SIMD path is restored.
foolnotion added 2 commits June 3, 2026 23:33
@foolnotion
refactor(concepts): rename ErrorMetric/Evaluator concepts to avoid na…
add9a8f 
…me clash

Rename Concepts::ErrorMetric -> Concepts::ErrorMetricCallable and
Concepts::Evaluator -> Concepts::EvaluatorCallable to prevent ambiguity
with the Operon::ErrorMetric struct and Operon::Evaluator class template
in any translation unit that does `using namespace Operon::Concepts`.

Also adds the weighted t(x, y, w) requirement to ErrorMetricCallable so
the concept fully describes the Operon::ErrorMetric callable interface.
@foolnotion
refactor(error_metrics): unify iterator constraints to std::contiguou…
a4ed4bc 
…s_iterator
@foolnotion
foolnotion merged commit 5f94ac0 into main Jun 4, 2026
3 checks passed
@foolnotion
foolnotion deleted the feature/concepts branch June 4, 2026 14:10
foolnotion added a commit that referenced this pull request Jul 13, 2026
@foolnotion
Merge pull request #67 from heal-research/feature/concepts
fb6102f 
feat(concepts): add C++20 concepts for operators, metrics, and policy types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@foolnotion