Skip to content

integrate_1d_gauss_kronrod performance fix - #3403

Open
avehtari wants to merge 5 commits into
developfrom
gk-boost-atol
Open

avehtari wants to merge 5 commits into
developfrom
gk-boost-atol

Conversation

@avehtari

@avehtari avehtari commented Sep 15, 2026

Copy link
Copy Markdown
Member

Fixes #3392 with minimal approach (not fixing minor atol behavior continuity issue and not fixing the wasted ajoint computation).

Summary

Boost's adaptive Gauss-Kronrod recursion already accepts an absolute-error budget and honours it in its leaf criterion — recursive_adaptive_integrate(info, a, b, max_levels, abs_tol, error, L1). Only the public integrate() entry point is missing a way to set it: it hard-codes Real(0) at all five call sites.

Two changes:

vendored Boost one trailing Real abs_tol = Real(0) parameter on gauss_kronrod<Real, N>::integrate(), forwarded at the five recursion entry points
Stan pass absolute_tolerance through — one line

The parameter is trailing and defaults to the value previously hard-coded, so every existing Boost caller is unaffected and their quadrature is bit-for-bit unchanged.

  1. Add abs_tol parameter to vendored Boost gauss_kronrod::integrate: the patch, STAN_CHANGES, .gitattributes
  2. Fix Gauss-Kronrod order in docs: the pair is (G10,K21), not (G21,K21): pre-existing typo, 7 occurrences.
  3. Apply absolute tolerance during Gauss-Kronrod refinement: the Stan line, documentation, tests.

absolute_tolerance == 0 is Boost's sentinel for "derive the refinement budget from the root panel's own relative target", not "no absolute floor". A positive but negligible value therefore removes that derived budget and can refine slightly more than zero does. That is in the safe direction.

Tests

prim 14, rev 18, mix 2, all passing.
positive_abs_tol_reduces_work_on_negligible_integrand doubles as the guard on the vendored patch: if absolute_tolerance ever stops reaching refinement, the two evaluation counts become equal and it fails. Also covered: work bounded on an unresolvable integrand, monotonicity in abs_tol, the throw still firing when unconverged, max_depth = 0, and all four limit transforms.

The saves in evals is already documented in #3392

Follow-ups, not in this PR

  • Maybe submit a PR upstream to boostorg/math?
  • integrate_1d_adjoint runs one quadrature per var scalar and does a full reverse sweep at each node while keeping a single adjoint. Batching them onto one shared partition is a further ~8× on this model and needs no additional Boost change, but it touches adjoint code shared with integrate_1d, so maybe another PR?

Side Effects

No

Release notes

  • integrate_1d_gauss_kronrod gradient performance improved.

Checklist

  • Copyright holder: Aki Vehtari

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
    - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
    - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

@avehtari

Copy link
Copy Markdown
Member Author

Argh, TIL, if the PR title is empty the PR message will be replaced by the template. Just a sec writing the PR message again

@avehtari

Copy link
Copy Markdown
Member Author

PR message fixed

@avehtari
avehtari requested a review from WardBrian September 15, 2026 19:26

@WardBrian WardBrian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks Aki. Mostly comments asking you to delete stuff or clean up some tests.

The saves in evals is already documented in #3392

Is the same table exactly true or does it just catch the general idea?

Comment thread lib/boost_1.87.0/STAN_CHANGES Outdated
Comment thread stan/math/prim/functor/integrate_1d_gauss_kronrod.hpp Outdated
Comment thread test/unit/math/prim/functor/integrate_1d_gauss_kronrod_test.cpp Outdated
Comment thread test/unit/math/prim/functor/integrate_1d_gauss_kronrod_test.cpp Outdated
Comment thread test/unit/math/prim/functor/integrate_1d_gauss_kronrod_test.cpp Outdated
@avehtari

Copy link
Copy Markdown
Member Author

Is the same table exactly true or does it just catch the general idea?

Excatly the same table, otherwise I would have shown the new table.

@avehtari

Copy link
Copy Markdown
Member Author

Trimmed docs and tests. Passing evaluation counters by reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

integrate_1d_gauss_kronrod gradient computation can be very slow

2 participants