Repository navigation
docs: bring the composable recurrences page up to API v0.1 - #32
Conversation
Redraw the package map with ComposableRecurrences as a foundation that ComposableTuringIDModels, ComposedDistributions and CensoredDistributions plan to build on, marked as planned. Drop the Reactant, EpiAwareADTools and distribution-stack boxes. Move the example, modifiers and status to the v0.1 API and what is merged on main. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
📄 Site preview builtThe rendered site for this PR is available as the site-preview artifact on this run. Download it and open |
![seabbs-review-bot[bot]](https://avatars.githubusercontent.com/in/4624046?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR rewrites the composable recurrences page for ComposableRecurrences API v0.1. It updates the example to Depletion, Add and with_state, and it redraws the package map as planned uses. The content is broadly consistent. I found an internal contradiction about Imports and Add, and some wording that narrates the API change. I did not run the example against the package.
Automated first pass by seabbs-review-bot (Claude sonnet), triggered by: first pass. Not a human review. Comment @seabbs-review-bot to ask for another pass: @seabbs any time, the author's agent once it has pushed changes. Add the no-review label to opt this PR out. Ping @seabbs with any questions.
| The core operators are merged on the main branch of ComposableRecurrences.jl. | ||
| These are `Recurrence`, `Convolution`, the `TimeVarying`, `PerStratum` and `Pairwise` wrappers, and the coupling and modifier interfaces. | ||
| The core operators and the built-in modifiers are merged on the main branch of ComposableRecurrences.jl. | ||
| These are `Recurrence`, `Convolution`, the `TimeVarying`, `PerStratum` and `Pairwise` wrappers, the coupling and modifier interfaces, and `Depletion`, `Imports`, `Redistribute` and `Clamp`. |
There was a problem hiding this comment.
suggestion The page names Imports as merged here, but the example, the modifiers section and the package map all use Add. Line 252 then says Imports is renamed to Add. A reader meeting Imports here will not find it in the v0.1 API the example uses. List Add here and drop the rename from line 252.
|
|
||
| The built-in modifiers (`Depletion`, `Imports`, `Redistribute`, `Clamp`) and the native Mooncake and Enzyme adjoints are in progress. | ||
| The API v0.1 consolidation is in progress. | ||
| It makes every option a struct, renames `Imports` to `Add`, makes `Pairwise` a kernel, and replaces `return_state` with `with_state`. |
There was a problem hiding this comment.
suggestion This paragraph is a changelog ("renames Imports to Add", "replaces return_state with with_state"). Readers of the page never saw the old names. Say what the API is, for example: "v0.1 makes every option a struct and makes Pairwise a kernel. A release follows once it and the native adjoints are done."
| ::: | ||
|
|
||
| The example runs on the main branch of [ComposableRecurrences.jl](https://github.com/EpiAware/ComposableRecurrences.jl). | ||
| The example uses API v0.1 of [ComposableRecurrences.jl](https://github.com/EpiAware/ComposableRecurrences.jl), which is landing on the main branch now. |
There was a problem hiding this comment.
suggestion "Which is landing on the main branch now" is time-relative and will go stale. It also conflicts with the Status section, which says the operators and modifiers are already merged. Say one thing in both places, for example "The example uses the v0.1 API of ComposableRecurrences.jl. Check the repository for which commit or release carries it."
|
|
||
| A `Recurrence` feeds its output back into its own history. | ||
| At each step it convolves the last `L` values with its kernel, mixes strata through a coupling, scales by a gain, adds an input, and passes the result through a tuple of modifiers. | ||
| An operator is built from structs, and those structs compose. |
There was a problem hiding this comment.
suggestion "An operator is built from structs, and those structs compose." says nothing the next paragraph and the modifiers section do not. Cut it.
| A `Recurrence` kernel starts at lag 1 and a `Convolution` kernel at lag 0, so nothing is ever reversed by hand. | ||
| Operators are built once, outside the model, and called with the per-day inputs that are usually sampled. | ||
| Chaining is function composition, as in `reports(infections(R; history))`. | ||
| Every time-indexed input is read at absolute time, so `with_state` returns the output with its state and a later call resumes from `state`. |
There was a problem hiding this comment.
suggestion This sentence joins two claims with "so". Absolute-time indexing does not explain with_state returning its state. Split it: "Time-indexed inputs are read at absolute time, so a resumed call needs the full series and not a slice. with_state returns the state to resume from." The example passes the full R to the resumed call, and the text should say why.
|
|
||
| ```{.julia .code-overflow-scroll} | ||
| using ComposableRecurrences | ||
| using ComposableRecurrences: Depletion, Floor, Add, with_state |
There was a problem hiding this comment.
suggestion The import line brings in Depletion, Floor, Add and with_state but not PerStratum. The example also uses Recurrence, Convolution and PerStratum. If these are not exported, the example fails as written, and if they are exported it is unclear why the four listed names are not. Confirm the export list and import consistently.
Updates the composable recurrences approach page.
Depletion(N, Floor()),Add(b),TimeVarying(x, Primary()),Pairwiseas a kernel andwith_statefor resume. One line notes that the API pass is landing now.This was opened by a bot. Please ping @seabbs for any questions.