Skip to content

Redesign bisection events, separate init from loop, extract shared types#270

Merged
dyreby merged 3 commits into
mainfrom
250-bisection-event-redesign
Mar 4, 2026
Merged

Redesign bisection events, separate init from loop, extract shared types#270
dyreby merged 3 commits into
mainfrom
250-bisection-event-redesign

Conversation

@dyreby

@dyreby dyreby commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

What

Bisection's event types wrapped &EvaluateResult, a private intermediate type that made events unconstructable in tests without driving the full solver. This blocked focused unit testing of observer logic and was the only solver with this limitation — golden section's events were already directly constructable.

This PR redesigns bisection's events to match the golden section pattern, separates endpoint initialization from the midpoint loop, and extracts shared bracketing types to the equation module level in preparation for ITP (#269).

Closes #250.

How

Event redesign — Replace the three phase-based variants (Left/Right/Midpoint, each carrying &EvaluateResult) with three outcome-based variants (Evaluated/ModelFailed/ProblemFailed) carrying plain public fields. Every event carries a &Bracket reference since events are only emitted during the midpoint loop.

Init/loop separation — Endpoint evaluation moves out of the observer protocol entirely, matching golden section's architecture. Two entry points:

  • solve takes [f64; 2], evaluates endpoints, builds a bracket, and runs the midpoint loop. Convenient for most callers. Endpoint failures are hard errors. Endpoints are automatically ordered.
  • solve_from_bracket takes a pre-validated Bracket and runs the midpoint loop directly. Use this when you need control over endpoint evaluation — e.g., domain-specific error recovery, noise filtering near bracket boundaries, or skipping redundant evaluations.

Bracket::new — Now public. Takes (f64, Sign) pairs for left and right. Left must be strictly less than right (no silent reordering). Signs pair structurally with their endpoints.

Shared type extractionBracket, Sign, Bounds, BracketError, Best, Solution, and Status move from bisection to the equation module. These are identical across bracketing root finders and will be reused by ITP. Bisection re-exports them to preserve its public API surface.

New types:

  • Point { x, residual } — equation analog to golden section's Point { x, objective }
  • BracketError::Inverted — for left > right

Migration

Previously, observers received Left/Right/Midpoint events and could recover from endpoint failures via AssumeResidualSign. Observers now only see midpoint events.

If your observer handled endpoint failures, evaluate endpoints yourself and use solve_from_bracket:

// Before: observer recovered from endpoint failures
let solution = bisection::solve(&model, &problem, [a, b], &config, observer)?;

// After: evaluate endpoints, build bracket, then solve
let left = equation::evaluate(&model, &problem, [a])?;
let right = equation::evaluate(&model, &problem, [b])?;
let bracket = Bracket::new(
    (a, Sign::of(left.residuals[0])),
    (b, Sign::of(right.residuals[0])),
)?;
let solution = bisection::solve_from_bracket(&model, &problem, bracket, &config, observer)?;

Testing

Observer tests in twine-observers now construct bisection events directly without driving the solver, matching the golden section test pattern.

Move Bracket, Sign, Bounds, BracketError, Best, Solution, and Status
from bisection to the equation module. These types are shared by all
bracketing root finders and will be reused by ITP (#269).

Best no longer wraps Option<Evaluation> internally. Callers hold
Option<Best> and create it from the first successful evaluation.
This makes the type's invariant clearer: a Best always has content.

Bisection re-exports the public types to preserve its API surface.
No behavior changes.
@dyreby dyreby force-pushed the 250-bisection-event-redesign branch from 1a249a6 to 9d12d5c Compare March 4, 2026 12:51
@dyreby dyreby changed the title Redesign bisection events and extract shared bracketing types Redesign bisection events, separate init from loop, extract shared types Mar 4, 2026
@dyreby dyreby force-pushed the 250-bisection-event-redesign branch from 9d12d5c to ae27ee6 Compare March 4, 2026 13:25
@dyreby dyreby requested a review from gdtroszak March 4, 2026 13:26
Replace the three phase-based event variants (Left/Right/Midpoint, each
carrying &EvaluateResult) with three outcome-based variants (Evaluated,
ModelFailed, ProblemFailed) following the golden section pattern.

Separate endpoint initialization from the midpoint loop, matching golden
section's architecture. This eliminates the Phase enum entirely — events
are always midpoint evaluations and always carry a &Bracket reference.

Add solve_from_bracket for callers who need control over endpoint
evaluation (e.g., domain-specific error recovery or noise filtering).
The standard solve evaluates endpoints internally and delegates to
solve_from_bracket for the midpoint loop.

Make Bracket::new public so callers can construct validated brackets
from endpoints and residual signs.

New types:
- Point { x, residual } — equation analog to golden section's Point
- Bracket::new is now public

Removed types:
- Phase — no longer needed with init/loop separation
- EvalContext endpoint methods — only midpoint remains
@dyreby dyreby force-pushed the 250-bisection-event-redesign branch from ae27ee6 to 35dfca4 Compare March 4, 2026 13:29

@gdtroszak gdtroszak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Clean separation between the two commits, and the event redesign delivers on #250's acceptance criteria. Two minor comments inline.

}

/// Emits a failure event and returns the observer's action.
pub(super) fn emit_failure<Obs>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider moving this to EvalContext or a free function in the eval_context module. Event is otherwise a pure data type (variants + accessors), and emit_failure adds observer dispatch responsibility. Not blocking — it works — but it couples event construction with observer coordination in a way that might feel odd as more methods land here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call — moved to EvalContext::observe_failure, Event is back to pure data. Doc nit addressed too. 7112098

/// 2. Validate that the endpoints bracket a root using residual signs.
/// 3. Iterate: evaluate the midpoint, shrink the bracket, and update the best evaluation.
/// Evaluates both endpoints to establish a bracket, then iterates by
/// evaluating midpoints and shrinking the bracket.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: might be worth mentioning the specific error variants returned for endpoint failures (Error::Model / Error::Problem) since the doc emphasizes that endpoint failures are now hard errors.

@dyreby dyreby merged commit a80ae31 into main Mar 4, 2026
1 check passed
@dyreby dyreby deleted the 250-bisection-event-redesign branch March 4, 2026 14:45
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.

Revisit bisection design in light of golden section

2 participants