Use A42 instead of ITP in find_alpha#482
Merged
Merged
Conversation
Roots 3.0.2 changed `decide_convergence` for bracketing methods to return `NaN` (raising `ConvergenceFailed`) when the algorithm terminates in a `:not_converged` state, instead of returning the best endpoint. `Roots.ITP` can stall a few floats short of the (unreachable, `eps^3`) x-tolerance and hits exactly this path, so `find_alpha` errored for many inputs. `Roots.A42()` (Alefeld-Potra-Shi) is superlinear like ITP but narrows the bracket down to adjacent floats and converges via the exact-x path, so it works identically on both old and new Roots. The analytic AD rules are unchanged: they depend only on the returned value being a root of the same equation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Bijectors.jl documentation for PR #482 is available at: |
Member
Author
|
@devmotion, can you help review this PR? |
Distributions 0.25.129's bounded kernel-density samplers (Biweight, Cosine, …) can return NaN, which propagated into the transform/logpdf/AD comparisons and failed the `Biweight` interface test. Draw via a `randfinite` helper that resamples until finite so the tests exercise Bijectors, not the upstream bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yebai
commented
Jul 8, 2026
| using Bijectors | ||
| Random.seed!(123) | ||
|
|
||
| # Distributions 0.25.129's bounded kernel-density samplers (Biweight, Cosine, …) can |
Member
Author
There was a problem hiding this comment.
This is a separate, unrelated test-fragility fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yebai
force-pushed
the
fix-find-alpha-roots-3.0.2
branch
from
d1f406a to
1ad685c
Compare
yebai
commented
Jul 8, 2026
| return "joint order statistic $(_name(d.dist)) with length $(length(d))" | ||
| end | ||
|
|
||
| # Some Distributions kernel-density samplers (Biweight, Cosine, …) return NaN in recent |
Member
Author
There was a problem hiding this comment.
Also, a separate, unrelated test-fragility fix.
Recent Distributions releases break some samplers: the bounded kernel-density distributions (Biweight, Triweight, Epanechnikov) return NaN, and Cosine's quantile-based sampler throws `Roots.ConvergenceFailed` under Roots 3. Both propagated into `test_all` (roundtrip, logjac, …) and failed the Vector and ReverseDiff suites. Draw samples via a `randfinite` helper that resamples until finite (retrying on NaN and on a thrown sampler error), matching the `randfinite` approach used in the univariate interface tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yebai
force-pushed
the
fix-find-alpha-roots-3.0.2
branch
from
1ad685c to
42f472c
Compare
Member
Author
|
I'm merging this as it (1) is fairly low risk, (2) it currently breaks other TuringLang tests. |
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.
Roots 3.0.2 made
find_zeroreturnNaN(→ConvergenceFailed) when a bracketing method stops in a:not_convergedstate instead of returning the best endpoint.ITPhits this by stalling short of the unreachableeps^3x-tolerance, breakingfind_alpha.A42is superlinear like ITP but narrows to adjacent floats and converges via the exact-x path, working on both old and new Roots. The analytic AD rules remain unchanged (they depend only on the returned value being a root).