Integrate with hints - #1
Conversation
Refactored integrate method to use addaptQuadrature for adaptive integration.
Refactor adaptive quadrature methods to improve clarity and reduce redundancy.
There was a problem hiding this comment.
Pull request overview
This PR refactors the finite-interval adaptive quadrature implementation by extracting the iterative “adaptation” loop into a reusable helper, likely as groundwork for alternative initialization strategies (e.g., hinted subdivision).
Changes:
- Extracted the core adaptive refinement loop into a new private helper (
addaptQuadrature). - Reworked
integrate(xmin, xmax)to only build the initial subdivision and then delegate refinement to the helper. - Minor local cleanup in the refinement loop (structured binding for the second interval estimate).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| include/LNIT/AdaptiveQuadratures/AdaptiveQuadratureBase.hpp | Declares a new private helper to run the adaptive refinement loop. |
| include/LNIT/AdaptiveQuadratures/AdaptiveQuadratureBase_impl.hpp | Moves the refinement loop into the helper and makes integrate delegate to it after initial subdivision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| template<class Derived> template<class Function> | ||
| auto AdaptiveQuadratureBase<Derived>::integrate(const Function& f, const Scalar& xmin, const Scalar& xmax) -> LongScalar | ||
| { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
include/LNIT/AdaptiveQuadratures/AdaptiveQuadratureBase_impl.hpp:125
- Range-for iterates mu as
const double&, which forces a conversion from Scalar to double and can lose precision (and breaks if Scalar isn’t implicitly convertible to double). Iterate using Scalar instead.
for (const double& mu_i : mu)
include/LNIT/AdaptiveQuadratures/AdaptiveQuadratureBase_impl.hpp:123
- If
muis empty, no sub-intervals are created and adaptQuadrature() will call max_element() on an empty error vector (UB). Consider falling back to the existing infinite-range integrate(f) behavior when no hints are provided.
m_intervals.clear();
m_subIntergrals.clear();
m_subIntergralsErr.clear();
m_intervals.reserve(2*mu.size());
include/LNIT/AdaptiveQuadratures/AdaptiveQuadratureBase_impl.hpp:179
fmt::print(..., "{}", m_intervals)requires fmt range formatting support (e.g., <fmt/ranges.h>) and a formatter for Interval/pair. With only <fmt/core.h> included here, this is likely to fail to compile. Consider logging only the number of intervals, or include the necessary fmt headers.
if (m_out) { fmt::print(m_out, "#NumericalIntegrator adapting quadrature over {}\n", m_intervals); }
| template<class Function> LongScalar integrate(const Function& f, const Scalar& xmin, const Scalar& xmax); | ||
|
|
||
|
|
||
| template<class Function> LongScalar integrateWithHints(const Function& f, const std::span<const Scalar> mu, const Scalar& sigma); | ||
|
|
There was a problem hiding this comment.
PR comment was fixed to a std::span of means and a single sigma.
The goal of this PR is to use adaptive quadrature over$\mathbb{R}$ with hints on where the most relevant features of the integrate functions are.
The user provides a$\mathbb{R}$ from theses informations.
std::spanof mean and a single variance corresponding to the function's peaks the adaptive quadrature then computes an initial subdivision of