Raise XArrayEmitter buffer_size defaults 3/4 -> 600 - #558
Merged
Conversation
The vendored viva-emitters XArrayEmitter flushes its in-memory buffer to the zarr store every `buffer_size` emit steps. v2ecoli was defaulting to 3-4, forcing a flush every few simulated seconds and degrading write latency and compression by ~2 orders of magnitude (per Boyan Beronov's analysis on vEcoli PR #414 / v2ecoli #506). The small default originated as a workaround for a `flush(final=True)` assertion in an early vendored emitter (buffer exactly full at close). That assertion is fixed in the viva-emitters `main` this repo installs (the buffer-full / partial / aligned close paths are regression-tested), so the small buffer is now pure overhead. Raise every caller default to 600, matching the viva-emitters library default (a handful of flushes per generation): - build_emitter_config: 4 -> 600 - run_multigen_xarray: 3 -> 600 (+ corrected the obsolete assert rationale) - ecoli_baseline._single_cell_xarray_config: 3 -> 600 - vivarium_ecoli_engine: drop hardcoded buffer_size=3 (inherit the default) - workflow/lineage: transducer-absent fallback 4 -> 600 The `except AssertionError` guards around the generation closes are left as belt-and-suspenders against the now-fixed quirk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cplong90
added a commit
that referenced
this pull request
Aug 21, 2026
…tion; defer the Millard grading
mbp-07 was built as the Millard kinetic-metabolism arc and read as "the Millard
study". It is really where engines OTHER than v2ecoli's own attach to the
cell-side interface contract, so it is now framed as the investigation's
extensibility demonstration. Three engines at three deliberately different
depths -- the ladder is the finding; flattening it to "three models drop in"
would overclaim:
1. Millard 2017 kinetic ODE -- the deepest. v2ecoli's tFBA Metabolism is
REMOVED and central-carbon metabolism runs through the kinetic ODE across
the whole-cell model's full process set (baseline_millard), then coupled to
the BiRD reactor. An ENGINE SWAP INSIDE THE WCM, not a standalone model.
⚠ The coupling is NOT symmetric: cell->reactor carries exchange, the
reverse direction is fixed by #550, and even after it this arm's medium
glucose is deliberately withheld so it cannot overwrite the model's
calibrated GLCx.
2. iML1515 genome-scale FBA -- graded against the same Beulig prefix, but
UNCOUPLED and partly fit to the reference: its uptake lower bounds come
FROM the measured Beulig rates, so growth is the only genuine prediction.
3. OxidizeME ME-model -- a NAMED candidate, not a wired one. Verified:
`pbg_oxidizeme` is in neither pyproject.toml nor uv.lock, is not
importable, and `local:OxidizeMEStep` does not resolve -- against a
positive control (`local:BiRDTransportProcess`) that does.
Acceptance criteria 35 -> 28. Withdrawn: the five Millard criteria, and the two
`wcm-*` criteria as REDUNDANT -- mbp-05 grades the identical `vs_beulig` card
across all five axis groups, so two were double-counted. mbp-07 retains the two
iML1515 gradings, which exist nowhere else in the workspace.
⚠ Honest arithmetic: five of the seven withdrawn rows were FAILING and two
passing, so the pass ratio improves as a side effect. Arithmetic, not
achievement. Roll-up {passing 14, caveats 5, failing 7, in-progress 2}.
★ Three review rounds. Round 3 found that round 2 had fixed the wrong surfaces:
- `purpose.question` is what BOTH published surfaces actually read for a study's
headline (the report template falls back to it; the dashboard tile takes its
first sentence). It still opened "The Millard kinetic-metabolism arc,
end-to-end ... 3-arm report-card comparison". Rewritten. The previous round's
annotations -- YAML comments, `at_a_glance`, `description`, `one_liner` --
render NOWHERE, and `one_liner` has no consumer at all. "Six surfaces made
coherent" was really two.
- ⛔ This edit broke the YAML a THIRD time, silently: an unquoted block-sequence
scalar containing ": " parses as a MAPPING, and the page renders it as a raw
JSON blob. Audit gate, lint and the full fast suite all passed over it.
Sweeping for the pattern found two more -- one pre-existing and already live
on the published page, one frozen into an explicit `? key : value` form by an
earlier dumper round-trip. All nine evidence entries are now strings.
- "~54 WCM processes" was inherited from a docstring. Measured 55 addressed
nodes in the committed composite-state; a reviewer counted 47 by a different
convention; the README says 55. The count is contested by counting rule, so
the number is dropped rather than defended on a quotable surface.
- `executive.verdict` still claimed the two architectural blockers were
"resolved 2026-05-29 by the EnvironmentMirror Step". They were not: the mirror
matched NONE of the coupler's compartment-tagged ids. Amended in place.
- ⚠ The "checks that cannot fail" rationale was self-serving.
`growth-recovered-vs-baseline` was GATED and is DE-promoted here, not merely
unpromoted; and `wcm-mass-conservation-closes-millard` (band [0.0, 0.05],
measured cumRel ~1.4) CAN discriminate and currently FAILS. Its absence from
the acceptance set is a real coverage gap, now stated as one.
Also corrected: `cell_side_interface_contract.md` claimed OxidizeME was
installed and auto-discovered (false on both halves; its status column also
still read "imported", the word that caused the misread). And mbp-01's RESOLVED
`env-driver-molecule-id-convention` said unmatched names are "silently skipped
-- fails closed"; the word "silently" is withdrawn deliberately, because that
silence is what let the reactor->cell defect survive.
⚠ DO NOT "tidy" mbp-07's baseline shape (noted in-file). Only study of 60 on a
top-level `baseline:` list; allowlisted for exactly that; the shapes are
mutually exclusive, and converting it moves it onto the shape the
observable-check readout guard cannot read (vivarium-workbench#913).
Depends on #550 for `_NOT_REACTOR_DRIVEN_SPECIES`, worded as pending.
Audit gate as CI runs it: exit 0, multiscale-bioprocess [pass], allowlist
unchanged at 15. Fast gate 1631 passed / 52 skipped / 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
★ Round 4 (fourth independent review). The recurring defect was never the
reasoning -- it was reasoning about the FILE instead of the PAGE, and fixing
only the surfaces the last reviewer named. Fixed by executing the renderers
against the real payload rather than inferring:
- The three-engine ladder rendered as THREE separate <ol>s each numbered "1.",
each followed by an orphan paragraph starting mid-sentence: the dashboard's
list branch consumes only lines that themselves match `^\d+\.`, so
continuation lines break the list. One logical line per item now. Verified by
running `_renderInvLeadMarkdown` on the actual payload: 1 <ol>, 3 <li>,
0 orphans.
- `purpose.question` IS the study headline on both surfaces, and it rendered as
2848 characters of italic run-on with the dashboard card cut mid-clause.
Now 1197 characters, first line a complete sentence.
- ⛔ The rendered fields asserted #550's effect as present fact while #550 is
UNMERGED; the "pending" hedge existed only in a YAML comment that renders
nowhere. That is the same error the verdict amendment exists to retract.
Hedged in the rendered text.
- "~54 WCM processes" survived in NEXT_STEPS.md and a comment after the body
claimed it was dropped. Removed; zero occurrences remain.
- Provenance prose had been prefixed INTO `source_ref`, corrupting a
machine-resolvable `study:behavior` identifier. Refs restored verbatim; the
prose moved to a sibling `note:` key.
⚠ The local fast gate is RED on this tree and it is not this change: rebasing
onto current main brought #556/#558, which modify `composites/_helpers.py` and
`ecoli_baseline.py`, so the shared ParCa cache's inputs_hash no longer matches
and 20 tests fail with StaleCacheError. Verified by stashing this diff and
reproducing the identical failure on the rebased tree alone. CI rebuilds the
cache on hash miss, so CI is the unaffected gate here. The cache is SHARED via
symlink across every v2ecoli worktree, so any lane rebasing onto main hits this
until it is rebuilt.
Audit gate as CI runs it: exit 0, multiscale-bioprocess [pass], allowlist
unchanged at 15.
cplong90
added a commit
that referenced
this pull request
Aug 21, 2026
…tion; defer the Millard grading (#552) mbp-07 was built as the Millard kinetic-metabolism arc and read as "the Millard study". It is really where engines OTHER than v2ecoli's own attach to the cell-side interface contract, so it is now framed as the investigation's extensibility demonstration. Three engines at three deliberately different depths -- the ladder is the finding; flattening it to "three models drop in" would overclaim: 1. Millard 2017 kinetic ODE -- the deepest. v2ecoli's tFBA Metabolism is REMOVED and central-carbon metabolism runs through the kinetic ODE across the whole-cell model's full process set (baseline_millard), then coupled to the BiRD reactor. An ENGINE SWAP INSIDE THE WCM, not a standalone model. ⚠ The coupling is NOT symmetric: cell->reactor carries exchange, the reverse direction is fixed by #550, and even after it this arm's medium glucose is deliberately withheld so it cannot overwrite the model's calibrated GLCx. 2. iML1515 genome-scale FBA -- graded against the same Beulig prefix, but UNCOUPLED and partly fit to the reference: its uptake lower bounds come FROM the measured Beulig rates, so growth is the only genuine prediction. 3. OxidizeME ME-model -- a NAMED candidate, not a wired one. Verified: `pbg_oxidizeme` is in neither pyproject.toml nor uv.lock, is not importable, and `local:OxidizeMEStep` does not resolve -- against a positive control (`local:BiRDTransportProcess`) that does. Acceptance criteria 35 -> 28. Withdrawn: the five Millard criteria, and the two `wcm-*` criteria as REDUNDANT -- mbp-05 grades the identical `vs_beulig` card across all five axis groups, so two were double-counted. mbp-07 retains the two iML1515 gradings, which exist nowhere else in the workspace. ⚠ Honest arithmetic: five of the seven withdrawn rows were FAILING and two passing, so the pass ratio improves as a side effect. Arithmetic, not achievement. Roll-up {passing 14, caveats 5, failing 7, in-progress 2}. ★ Three review rounds. Round 3 found that round 2 had fixed the wrong surfaces: - `purpose.question` is what BOTH published surfaces actually read for a study's headline (the report template falls back to it; the dashboard tile takes its first sentence). It still opened "The Millard kinetic-metabolism arc, end-to-end ... 3-arm report-card comparison". Rewritten. The previous round's annotations -- YAML comments, `at_a_glance`, `description`, `one_liner` -- render NOWHERE, and `one_liner` has no consumer at all. "Six surfaces made coherent" was really two. - ⛔ This edit broke the YAML a THIRD time, silently: an unquoted block-sequence scalar containing ": " parses as a MAPPING, and the page renders it as a raw JSON blob. Audit gate, lint and the full fast suite all passed over it. Sweeping for the pattern found two more -- one pre-existing and already live on the published page, one frozen into an explicit `? key : value` form by an earlier dumper round-trip. All nine evidence entries are now strings. - "~54 WCM processes" was inherited from a docstring. Measured 55 addressed nodes in the committed composite-state; a reviewer counted 47 by a different convention; the README says 55. The count is contested by counting rule, so the number is dropped rather than defended on a quotable surface. - `executive.verdict` still claimed the two architectural blockers were "resolved 2026-05-29 by the EnvironmentMirror Step". They were not: the mirror matched NONE of the coupler's compartment-tagged ids. Amended in place. - ⚠ The "checks that cannot fail" rationale was self-serving. `growth-recovered-vs-baseline` was GATED and is DE-promoted here, not merely unpromoted; and `wcm-mass-conservation-closes-millard` (band [0.0, 0.05], measured cumRel ~1.4) CAN discriminate and currently FAILS. Its absence from the acceptance set is a real coverage gap, now stated as one. Also corrected: `cell_side_interface_contract.md` claimed OxidizeME was installed and auto-discovered (false on both halves; its status column also still read "imported", the word that caused the misread). And mbp-01's RESOLVED `env-driver-molecule-id-convention` said unmatched names are "silently skipped -- fails closed"; the word "silently" is withdrawn deliberately, because that silence is what let the reactor->cell defect survive. ⚠ DO NOT "tidy" mbp-07's baseline shape (noted in-file). Only study of 60 on a top-level `baseline:` list; allowlisted for exactly that; the shapes are mutually exclusive, and converting it moves it onto the shape the observable-check readout guard cannot read (vivarium-workbench#913). Depends on #550 for `_NOT_REACTOR_DRIVEN_SPECIES`, worded as pending. Audit gate as CI runs it: exit 0, multiscale-bioprocess [pass], allowlist unchanged at 15. Fast gate 1631 passed / 52 skipped / 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> ★ Round 4 (fourth independent review). The recurring defect was never the reasoning -- it was reasoning about the FILE instead of the PAGE, and fixing only the surfaces the last reviewer named. Fixed by executing the renderers against the real payload rather than inferring: - The three-engine ladder rendered as THREE separate <ol>s each numbered "1.", each followed by an orphan paragraph starting mid-sentence: the dashboard's list branch consumes only lines that themselves match `^\d+\.`, so continuation lines break the list. One logical line per item now. Verified by running `_renderInvLeadMarkdown` on the actual payload: 1 <ol>, 3 <li>, 0 orphans. - `purpose.question` IS the study headline on both surfaces, and it rendered as 2848 characters of italic run-on with the dashboard card cut mid-clause. Now 1197 characters, first line a complete sentence. - ⛔ The rendered fields asserted #550's effect as present fact while #550 is UNMERGED; the "pending" hedge existed only in a YAML comment that renders nowhere. That is the same error the verdict amendment exists to retract. Hedged in the rendered text. - "~54 WCM processes" survived in NEXT_STEPS.md and a comment after the body claimed it was dropped. Removed; zero occurrences remain. - Provenance prose had been prefixed INTO `source_ref`, corrupting a machine-resolvable `study:behavior` identifier. Refs restored verbatim; the prose moved to a sibling `note:` key. ⚠ The local fast gate is RED on this tree and it is not this change: rebasing onto current main brought #556/#558, which modify `composites/_helpers.py` and `ecoli_baseline.py`, so the shared ParCa cache's inputs_hash no longer matches and 20 tests fail with StaleCacheError. Verified by stashing this diff and reproducing the identical failure on the rebased tree alone. CI rebuilds the cache on hash miss, so CI is the unaffected gate here. The cache is SHARED via symlink across every v2ecoli worktree, so any lane rebasing onto main hits this until it is rebuilt. Audit gate as CI runs it: exit 0, multiscale-bioprocess [pass], allowlist unchanged at 15.
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.
Why
The vendored viva-emitters
XArrayEmitterflushes its in-memory buffer to the zarr store everybuffer_sizeemit steps. v2ecoli defaulted to 3–4, forcing a flush every few simulated seconds and degrading write latency + compression by ~2 orders of magnitude. Diagnosed by Boyan Beronov on vEcoli PR #414 / v2ecoli #506.The small default originated as a workaround for a
flush(final=True)assertion in an early vendored emitter (buffer exactly full at close). That assertion is fixed in the viva-emittersmainthis repo installs — the buffer-full / partial / aligned close paths are all regression-tested there — so the tiny buffer is now pure overhead.What
Raise every caller default to 600, matching the viva-emitters library default:
build_emitter_config: 4 → 600run_multigen_xarray: 3 → 600 (+ corrected the now-obsolete assert rationale in the docstring)ecoli_baseline._single_cell_xarray_config: 3 → 600vivarium_ecoli_engine: drop hardcodedbuffer_size=3(inherit the default)workflow/lineage: transducer-absent fallback 4 → 600The
except AssertionErrorguards around generation closes are kept as belt-and-suspenders against the now-fixed quirk.Pairs with
viva-emitters#29 (library-side default + docs). This PR works against the currently-pinned viva-emitters
mainregardless, since 600 is passed explicitly.Verification
Imports clean; all builders now produce
buffer.size = 600. Existing tests that passbuffer_sizeexplicitly (colony/upstream-division/workflow) are unaffected.🤖 Generated with Claude Code