Skip to content

Review the run-record design and the skeleton #4

Description

@SimonHeybrock

The design changed on 2026-09-23. Run records now hold supplied intermediates, a template is the one partial request, and a sum over runs is a member stage per run plus a finalize stage. This replaces #1, whose items name files and decision numbers that no longer exist. Everything is on architecture-sketch. Bugs found so far are in #2, and stories that disagree with the design are in #3.

A counter-proposal is on branch 4-sums-as-list-parameters, stacked on architecture-sketch: a sum over runs is one run whose run parameter is a list, and a record always holds a whole configured pipeline. Items marked branch: below are answered, removed, or changed there.

Review, in this order

  • The counter-proposal, docs/developer/proposals/sums-as-list-parameters.md on 4-sums-as-list-parameters (about 15 min). Its costs and four open questions are at the end.
  • The proposal, docs/developer/proposals/run-records.md (about 20 min): the shortest complete picture, with code for each case.
  • The user-story tests, packages/essapps/tests/stories/, read next to docs/developer/user-stories.md. For each story: is this the API we want? The small stories in section S are the quickest start. branch: S6 and B1 pass.
  • The API surface: records.py (Template with cut and fill, RunRequest, Accumulate) and client.py (Client.request, Client.run). branch: Accumulate and supplied intermediates are gone; vary is a hint given at submission and no longer on RunRequest; apply returns a Group that carries the template's blanks.
  • Series: apply and _finalize in batch.py. A finalize lists every current member and is made from the rule's template. branch: no finalize; each arrival submits one request over every current run of the series.
  • Validation and identity: validate, _as_recorded, _shape_errors, _check_ref (the agreement check), and provenance in backend.py. branch: no agreement check and no workflow ID; every request is checked against the whole params model.
  • Tests written by agents, read critically: aggregation_test.py, stages_test.py, and the series tests in batch_test.py.
  • The docs: architecture.md, then "A growing series" in aggregation.md and "Series" in rules.md, then the other topic docs as needed. branch: the topic docs are not updated yet.

Carried over from #1 and not yet reviewed:

  • The LoKI notebooks read as a user (loki-session.ipynb, loki-batch.ipynb). Does any step feel wrong? A wrong step there is a design problem, not a code problem.
  • Rules, lookups, and the trigger loop against practice. They were written from reading Mantid and FIA, not from use. Are apply, backlog, reprocess, and retry the operations a scientist asks for?
  • Aggregation beyond SANS. Is "exposed intermediates that add, normalisation after" right for powder diffraction and BIFROST? branch: the accumulation keys stay inside the run and need not be exposed; the question becomes whether a member parameter with an accumulator fits.
  • The workflow contract (binding.py, adapter.py). Is it how we would bind DREAM or BIFROST, and does it live in ess.reduce next to the spec or in essapps? Instrument packages cannot ship bindings until this is decided.
  • The findings from binding LoKI and Amor, in open-issues.md under "What binding real workflows found": decide which to fix and which to dismiss.

Decide

  • Sums as list parameters, or a reads table on the spec. On architecture-sketch the backend does not know which parameters a stage reads, and four things follow. The two-member-table case is refused. A stage cut at an intermediate is validated only at run time. A record's params mixes values the run read with values the cut made irrelevant. vary, documented as a hint, decides the workflow ID and what the agreement check skips. The option under S6 in User stories that disagree with the design #3 gives the backend the graph as a reads table. The counter-proposal removes the need: records never hold a piece of a pipeline, and only the binding, which has the graph, decides what a session holds. Its main cost: the runs of one request over a list do not reduce in parallel across processes. A client can recover that by chaining two specs over references, but a rule cannot.
  • A rule over a stitch. Series only accumulates, so a reflectometry angle series under a rule has no clause (story E1 is a question). Confirm this, or add a series clause that names a spec over a list of references. branch: answered; a series submits one request over a list of runs, so a stitch needs only a spec over a list of runs, which ess.apps.amor does not have yet.
  • How strict a series is about shared values. A member with a pinned value, or with a lookup fill on a field the template also sets, disagrees with the finalize and is refused. Where that boundary lies changes with the first decision. branch: a series is one request, so a lookup must fill every run of a series alike, and apply refuses the series otherwise.
  • intermediates, and reads if adopted, upstream in ess.reduce.spec ([ESSREDUCE] Minimal implementation-independent workflow specifications ess#690). branch: only intermediates.
  • Delete the proposals once reviewed, since they overlap the topic docs.

Smaller points

  • reused on a finalize cannot show whether a held accumulator served it. branch: no finalize.
  • With sciline.Buffered a finalize holds every contribution in memory, and nothing warns the author of an event-mode series. branch: unchanged, for the stage that holds the accumulation.
  • essapps submit can express only plain runs.
  • No query uses the index on the store's workflow column. branch: the column is gone with the workflow ID.
  • The sciline adapter probes a Stage to find which varied parameters the outputs need, because sciline refuses an input that is not needed. Changing that in sciline would delete the probe.
  • The markdown of loki-session.ipynb still cites old decision numbers (D7, D9).

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions