Progress reporting overhaul (1.8.0)#32
Merged
Merged
Conversation
R's SIGINT handler only sets a pending flag; the longjmp fires at the next explicit R_CheckUserInterrupt() call. Without it, the longjmp can fire asynchronously inside wait_timeout, skipping Rust destructors and corrupting the session. Now poll every 200ms unconditionally (was 600s when no progress callback) and call R_CheckUserInterrupt() at each timeout wakeup — a clean boundary where no Rust resources are mid-use. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous placement (after cb.call) was ineffective: R's evaluator consumes the interrupt flag internally when it throws during callback evaluation. By the time R_CheckUserInterrupt() ran, the flag was already cleared and it was a no-op — sampling never stopped. Moving the check before any R re-entry means the longjmp fires at a clean Rust boundary before the interrupt can be swallowed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rather than calling R_CheckUserInterrupt() (which longjmp's past the assignment, leaving the user with a confusing "object not found" on the next line), peek at R_interrupts_pending directly, clear it, and return a normal Err. extendr converts this to an R stop() at the nutpie_sample() call site with just "Sampling interrupted." and nothing after. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace adaptive two-pass status assembly with a stable fixed layout: always shows `div: N | X.X grad/draw`, positionally consistent across the full run. - ⚠/! prefix on div count when divergences > 0 (cli::symbol$warning with plain-text fallback) - ~ prefix on grad/draw when avg leapfrog >= 5% of 2^max_treedepth-1, i.e. treedepth cap is being hit with meaningful frequency - Color: red for divergences, yellow for elevated grad/draw - Add | separator before status field in CLI format string for visual consistency with the bar's other | delimiters - Remove chain-lag logic, adaptive-width two-pass, format_hard_chain_status, format_chain_progress_status, format_draw_range, format_chain_fraction_compact (all dead after the layout change) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add environment-aware progress routing: Positron and proper ANSI terminals (Ghostty, iTerm2, etc.) get one cli progress bar per chain; RStudio falls back to the single aggregated bar; non-interactive R gets plain text. The "multi" mode is also available as an explicit progress = "multi" option (falls back to "cli" with a warning if the terminal can't support it). Per-chain bars show each chain's own phase, percent, draw count, ETA, div count, and grad/draw — no aggregation needed, so slow chains are immediately visible by position. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace ~ with ▲ for treedepth warnings. Add format_status_tokens()
with {div}/{grad}/{draws}/{lag}/{step} placeholders so callers can
customize the progress bar status field via chain_format=. Add one-time
cli_alert_info hints above the bar when divergences or treedepth caps
first appear. Add make_text_progress_callback() for 1-line-per-chain
scrolling output (replacing the Rust-side text mode). Remove "multi"
progress mode and is_multi_bar_capable()/make_multi_chain_progress_callback()
entirely. "auto" now routes to "cli" or "text" only.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
phase_label in mixed state now shows "sample N/M" (chains that have graduated to sampling) instead of "warmup N/M" (chains still tuning) — positive framing, shows progress rather than remaining work. Treedepth hint now includes the actual numbers that triggered it: "▲ grad/draw: avg X.X leapfrog steps/draw (≥N% of 2^D−1=M cap)" so users immediately see how saturated the treedepth budget is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Locks the design from review: teach-once-then-abbreviate hints (div,
grad/draw, spread), exact-data triggers with late-warmup baselines,
sticky spread token, opt-in gap-from-leader {spark}, progressr removal,
cli to Imports, message() stream unification, and the review bug list.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cli becomes a hard dependency (Suggests -> Imports), so every
requireNamespace("cli") conditional and non-cli fallback branch is
removed. progressr mode is dropped entirely: the choice, the
resolve_progress_mode() branch, make_progressr_callback(), and
in_with_progress() all go, along with progressr from Suggests.
Routing now depends only on environment: auto -> cli when
interactive() and not rendering a knitr document, else text.
Step 1 of progress-ui-plan.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With progress="text" now driven entirely by the R callback (a later step), the Rust-side per-chain text log is dead: R always passes a progress callback unless progress="none". Remove the rprintln chain lines, start banner, completion/divergence summary, last_reported and chain_announced bookkeeping, and announce_finished. The 200ms poll loop and interrupt check stay — they're needed in every mode, including "none" (no callback), so Ctrl-C still aborts. The now-unused refresh parameter is dropped from sample_stan, run_with_settings, and run_sampler; num_chains/num_draws/num_warmup become dead in the sampler helpers and are dropped there too (still used by configure_settings in sample_stan). extendr-wrappers.R updated to match the new sample_stan arity. Step 2 of progress-ui-plan.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Text-mode per-chain lines switch from cat() (stdout) to message() (stderr) so they interleave correctly with hints and the end summary, and so suppressMessages() silences the whole progress stream at once. Since the Rust start banner was removed in the previous step, the text branch now prints "Sampling N chains, M draws each (W warmup)" from R before sampling. The cli bar still carries its own header. Update the text-callback test to capture_messages() instead of capture.output(). Step 3 of progress-ui-plan.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nuts-rs 0.18.2 already records divergences only outside tuning, so the
live {div} token, the hint, and red styling are post-warmup-only with no
extra filtering — the previously planned `>= num_warmup` filter is a
no-op. The remaining real fix: first_divergence in
summarize_progress_snapshot now reports a sample-relative, 1-indexed
draw (subtract num_warmup, +1) to match nutpieR's diagnostics
convention; the divergent_draws indices from nuts-rs are warmup-inclusive
and 0-based.
Introduce a small shared one-shot hint constructor (new_progress_hints)
used by both the cli and text callbacks, plus emit_progress_hint (cli
mode -> cli_alert_*, the !/i symbols come free; text mode -> message()).
The divergence hint is reworded to "div: divergent transitions
detected — try increasing `target_accept` or reparameterizing." and now
fires in text mode too, exactly once per run.
Step 4 of progress-ui-plan.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a tunable constants block at the top of progress.R
(LATE_WARMUP_FRACTION, GRAD_HINT_THRESHOLD, CAP_SUMMARY_THRESHOLD, and
the spread constants used in the next step).
grad/draw now reports a late-warmup-baseline average: when a chain first
passes LATE_WARMUP_FRACTION of warmup, its (total_num_steps,
finished_draws) is snapshotted; the pooled average is
sum(total - base) / sum(finished - base) over baselined chains — exact,
unbiased, no cap detection. The cli {grad} token switches to this
average once baselines exist so the bar and the hint never disagree.
format_gradient_status accents the token (▲, or ^ when the console isn't
UTF-8) once the average reaches GRAD_HINT_THRESHOLD, an absolute
leapfrog-steps/draw count rather than a fraction of the treedepth cap.
New informational grad/draw hint (cli + text), fired once, with tree
depth round(log2(avg+1)). max_treedepth advice deliberately stays out of
the hint and lives only in the end summary, which now reports the exact
%-at-cap (depth >= max_treedepth, or n_steps >= 2^max_treedepth - 1) and
prints an advice line above CAP_SUMMARY_THRESHOLD.
Step 5 of progress-ui-plan.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… rename
Split the two chain-spread surfaces that the working-tree experiment had
conflated:
- {spread} is now a percent-range token, "spread 23-78%" (ASCII hyphen,
text-safe; percent not draw counts so it can't be confused with the
bar's current/total). It is empty until the callback latches it on, and
then renders for the rest of the run, collapsing honestly (e.g.
"spread 98-100%") as chains converge. It joins the default cli
chain_format: "{div} | {grad} | {spread}".
- {spark} is the opt-in gap-from-leader sparkline (the experiment's
glyph encoding), renamed from format_chain_spread() to
format_chain_spark(); never in the default format.
The trigger is computed over *started* chains only (so queued chains
when cores < num_chains don't fake a spread): max - min >=
SPREAD_TRIGGER_POINTS and the median chain past SPREAD_MEDIAN_FLOOR.
On latch, a one-shot cli-only spread hint fires. Revert the experiment's
always-on bar-format change: the bar shows pb_current/pb_total again and
spread/spark surface via the status field, not permanent bar real estate.
format_chain_draw_range switches its en dash to a hyphen; the empty-token
pipe cleanup in format_status_tokens becomes multi-pass so a run of three
empty tokens collapses cleanly.
Step 6 of progress-ui-plan.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- as_progress_num() guards the is.na() check with x[[1]] so a length > 1
input can't error ("condition has length > 1").
- Delete the dead print_progress_summary() (superseded by
print_sampling_diagnostic_summary()).
- chain_format roxygen documents the new token set ({div} post-warmup,
{grad} accent, {spread}, {spark}) and the new default.
- NEWS.md: user-facing 1.8.0 entry (modes, hints, tokens, cli Import,
progressr removal). Bump DESCRIPTION to 1.8.0 — removing the
progress="progressr" option is a breaking API change; adjust the number
if your release cadence differs.
- CLAUDE.md: correct the stale nuts-rs 0.17.x note to 0.18.x, and replace
the "load_all doesn't work" note with the actual loop (devtools::test
works; full-suite debug panic in compile-cache is an extendr-0.9.0
artifact; every install is a full recompile).
(Bug fixes 1/4/5 from the review — multi-pass pipe cleanup, knitr guard,
▲ UTF-8 gating — landed in earlier steps.)
Step 7 of progress-ui-plan.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dogfooding feedback:
- Hints now emit via message() with cli::symbol$warning (⚠) / symbol$info
(ℹ) instead of cli_alert_warning/info, so the divergence hint shows the
same ⚠ as the {div} status token (cli_alert_warning is hardwired to "!",
which mismatched the bar). One glyph vocabulary across bar and hints, and
emission is now identical in cli and text modes — both on stderr. The
now-unused `mode` field drops off new_progress_hints().
- grad/draw hint reworded to lead with the problem and save a line:
"high (~129) gradient evaluations per draw (tree depth ~ 7) — often a
sign of difficult posterior geometry; worth a sanity check if
unexpected." The redundant "grad/draw:" label is gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep the "high (~N)" wording and tightened sentence, but re-add the "grad/draw:" label so all three hints (div:, grad/draw:, spread:) tie back to their status-line token uniformly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix the bar-trail regression: switching hints to plain message() lost cli's coordination with the active progress bar, so each hint stranded the current bar line. Hints now route through cli::cli_progress_output() when a cli bar is active (clears the bar, prints the hint above it, redraws one bar line), keeping the ⚠/ℹ glyphs. Falls back to message() in text mode or if the bar id is unusable (also keeps the id="fake" tests green and makes production robust). Divergence hint now states the stakes in a few words: "these can bias your results". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Drop the now-vestigial max_treedepth param from format_status_tokens
and summarize_progress_snapshot — neither reads it anymore (the {grad}
token switched to the absolute GRAD_HINT_THRESHOLD).
- fraction_at_treedepth_cap prefers nuts-rs's own maxdepth_reached flag
(the same field print.nutpie_diagnostics reports), so the %-at-cap line
and the diagnostics print can't disagree; depth / n_steps stay as
fallbacks when the flag is absent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two /simplify cleanups over the polish commits: - sampling_summary_table no longer returns a `chains` element; its only caller stopped reading it once the per-chain divergence loop folded into the table builder. - The treedepth-cap percentage was reported twice in the no-divergence path (headline plus a trailing info line). Fold the advice into the headline and gate the standalone cap line on `total_divs > 0L`, so it fires only for the divergence headlines that don't mention the cap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mingw (x86_64-pc-windows-gnu) toolchain does not auto-emit -lcrypt32 the way MSVC does, so the Cert* symbols pulled in by rustls-platform-verifier / ureq (via bridgestan's download-bridgestan-src feature) failed to link on the r-universe Windows build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ASCII-escape the in-code glyph literals (triangle accent, sparkline blocks) and em-dashes in user-facing strings; comment em-dashes are left as-is (permitted by the check). - Qualify tail() as utils::tail() to drop the undefined-global NOTE. - Make resolve_progress_mode() take an injectable use_cli so the dispatch can be tested without mocking base::interactive(), which does not take effect under R CMD check's non-interactive test_check() run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # DESCRIPTION # NEWS.md
Closed
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.
This PR improves the progress reporting considerably, aiming to provide an informative, opinionated experience similar to
nutpie's, within the constraints of R (most notably limited environment support for cursor up, which rules out multi-line progress bars).cliis used for the default mode, exposing:Also offered are:
chain_formatif you prefer different monitoring tools like a sparkline, or have stricter space requirements. In addition to the default displays, there are opt-in tokens such as {spark}, {draws}, {lag}, {step}, and {tdepth}.Here's a rough idea of what these signals look like during sampling of a problematic model; the goal is to help people catch issues early in runs where possible:
I experimented with
progressrfor a while, but was ultimately dissatisfied with the flexibility and stability it offered, so that approach has been depricated. Some of the issues in this CmdStan PR may be helpful for intuition there. Similarly, I considered a multi-line UX closer to nutpie's for supported environments, but decided they're relatively rarely used in the R ecosystem, so decided that wasn't worth the squeeze.This PR also bundles a fix for a minor windows build issue.
Testing
I tested this in most common R environments including positron, Rstudio, Quarto notebooks, and terminal.