Repository navigation
Require barry >= 0.2.2 (hash-collision fix) and make init_defm interruptible - #8
Merged
Merged
Conversation
barry 0.2.2 (USCbiostats/barry#24) fixes a catastrophic hash-collision bug that made init_defm() appear to hang for models with many outcome columns: all support statistics landed in a single unordered_map bucket, degrading support enumeration to O(4^ncol). A 16-outcome, order-1 model that previously never finished initializing now takes seconds. The LinkingTo requirement ensures defm is compiled against fixed headers. This also defines BARRY_USER_INTERRUPT as Rcpp::checkUserInterrupt() in every translation unit that includes barry, enabling barry's built-in interrupt checkpoint (every 1000 support iterations). Long-running calls such as init_defm() can now be stopped with Ctrl-C from an interactive session instead of requiring a force-kill. Verified via setTimeLimit(), which uses the same R_CheckUserInterrupt checkpoint: a ~20s init aborts at the 3s limit with a clean R error and no crash. A model interrupted mid-initialization should be rebuilt before further use (same caveat as any error thrown during init). All 25 tinytest results pass against barry 0.2.2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kage lands on CRAN)
CI could not resolve `barry (>= 0.2.2)`: CRAN only has 0.2.1, and pak's `deps::.` (used by setup-r-dependencies) does not consume the DESCRIPTION `Remotes:` field when a satisfying-named package exists on CRAN, so the GitHub source for the newer barry was never considered. Add the barryr PR branch as an explicit `extra-packages` entry in every workflow that installs dependencies. The action passes extra-packages straight to `pak::lockfile_create()`, which does honor an explicit remote spec; verified locally with pak 0.10.0 in a clean library that the graph now solves with barry -> 0.2.2 from the branch. All three entries are temporary and must be removed once barry 0.2.2 is on CRAN (tracked via USCbiostats/barryr#8); each is commented as such. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thank you for your contribution @gvegayon 🚀! The pkgdown site preview is ready for review 👉 Download here 👈! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8 +/- ##
=======================================
Coverage 81.37% 81.37%
=======================================
Files 8 8
Lines 451 451
=======================================
Hits 367 367
Misses 84 84 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
USCbiostats/barryr#8 is merged, so barryr's main branch is barry 0.2.2. Drop the PR-branch ref from the extra-packages remotes; plain USCbiostats/barryr now resolves to 0.2.2 and won't break when the branch is deleted. Verified with pak 0.10.0 in a clean library: the graph solves with barry -> 0.2.2 from barryr main (commit fdd41d5). Still temporary: remove these extra-packages entries entirely once barry 0.2.2 reaches CRAN. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
Companion to USCbiostats/barry#24 and USCbiostats/barryr#8, which fix a catastrophic hash-collision bug in barry's
FreqTable/vecHasher: support statistics (small-integer doubles) produced hashes with no entropy in their low bits, sounordered_mapchained every key in a single bucket and support enumeration degraded from O(2^ncol) to O(4^ncol). In defm terms,init_defm()on a 16-outcome, order-1 panel model appeared to hang forever (~14 s per unique Markov state before, ~0.05 s after; a 20-state synthetic case went from >5 min to under a second). This PR:barry (>= 0.2.2)inLinkingToso defm is compiled against the fixed headers (verified:R CMD INSTALLcorrectly refuses to build against barry 0.2.1).BARRY_USER_INTERRUPTasRcpp::checkUserInterrupt()in every translation unit that includes barry, enabling barry's built-in interrupt checkpoint (every 1000 support iterations). Longinit_defm()calls can now be stopped with Ctrl-C instead of force-killing R — this was the other half of the original report ("computer hangs and Ctrl-C does nothing").Verification
setTimeLimit(elapsed = 3), which fires at the sameR_CheckUserInterruptcheckpoint as Ctrl-C: a ~20 sinit_defm()aborts at ~3 s with a clean R error (reached elapsed time limit), no crash or corruption on unwind. PlainRscriptignores SIGINT entirely (verified with a pure-R loop baseline), so the time-limit route is the reliable non-interactive proxy.init_defm()is left partially initialized and should be rebuilt withnew_defm()before reuse — same caveat as any error thrown during init (documented in NEWS).Merge order: needs barry ≥ 0.2.2 available (USCbiostats/barry#24 → USCbiostats/barryr#8 → this); CI will fail to satisfy
LinkingTountil the barryr release is on CRAN/r-universe.🤖 Generated with Claude Code