Skip to content

core: simulation scheduling fixes + buffer accounting - #117

Merged
cweniger merged 4 commits into
mainfrom
fix/core-sim-scheduling-and-buffer-accounting
Aug 8, 2026
Merged

core: simulation scheduling fixes + buffer accounting#117
cweniger merged 4 commits into
mainfrom
fix/core-sim-scheduling-and-buffer-accounting

Conversation

@cweniger

@cweniger cweniger commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Four independent core fixes extracted from plan/gaussianized-flow-matching, where they were developed alongside the GFM estimator. None of them touch estimator code, so they can land ahead of that work.

Each commit stands alone and is individually revertable.

Commits

1. core: zero-sample dispatch guards + native-int num_resims (cherry-pick of d46aac4)

Crash fix. When n_samples < num_actors, MultiplexNodeWrapper._multiplexed_call hands some actors an empty index slice. That reaches _resolve_refs, where an empty ref list falls past the broadcast check into np.stack([]) and dies with an unhelpful need at least one array to stack.

Idle actors are now skipped, and _resolve_refs gained an explicit ValueError as a backstop that names the actual cause. Also converts num_resims() from np.int64 to a native int.

No behaviour change on the normal path.

2. core: report the actual trainable pool in buffer/n_training (extracted from e712a3e)

Metric fix, logging only. buffer/n_training counted only SampleStatus.TRAINING, but BufferView.cached_loader draws from [TRAINING, DISFAVOURED] and rotate_sample_buffer floors that sum at min_samples. The metric therefore read below min_samples as soon as anything was disfavoured, even though those samples are still being trained on until tombstoned.

n_disfavoured is unchanged and still shows the eviction-pending subset, so the split stays visible.

3. core: stop the ray.wait timeout from inflating simulate_interval (from 6f4c6bb)

⚠️ Behaviour change — please read.

The launch loop paces itself with time.sleep(simulate_interval), but each iteration first sat in ray.wait(..., timeout=1). The real cadence was simulate_interval + 1s. At the common simulate_interval=1 that halved the simulation rate relative to the configured value.

The poll timeout drops to 0.05s so simulate_interval means what it says. Existing configs will simulate roughly 2x faster at simulate_interval=1, converging to no change as simulate_interval grows. Anything tuned against the old effective cadence wants simulate_interval raised by ~1s to match.

Side effect: during a graceful stop the sleep is skipped, so the loop polls at 50ms rather than 1s while training winds down.

4. core: overlap the buffer append with the next simulation round (from 6f4c6bb)

append_refs() was awaited synchronously, stalling the launch loop on the dataset-manager actor (a ray.put per sample, plus rotate_sample_buffer) before the next simulation round could start. Now one append stays in flight, with a flush at loop exit so nothing is lost at shutdown.

Consequence: num_resims() is queried before the previous append lands, so the buffer can transiently exceed max_samples by up to one simulate_count batch. rotate_sample_buffer() tombstones the excess on the next append, so the steady state is unchanged. Exceptions from append_refs now surface one iteration later.

Commits 3 and 4 were one commit upstream; they are split here because 3 changes observable behaviour and 4 does not, so 3 should be bisectable and revertable on its own.

Testing

Not run — import torch fails in my environment (libcudnn.so.9: cannot open shared object file), which also breaks pytest tests collection on main, so it is not something this branch introduces. Verification so far is static review plus py_compile.

tests/test_examples_smoke.py should be run on a GPU node before merge, mainly to exercise commit 3's timing change end to end.

🤖 Generated with Claude Code

cweniger and others added 4 commits August 8, 2026 22:57
- _multiplexed_call skips idle actors when n_samples < num_actors leaves
  them an empty slice (previously crashed downstream).
- _resolve_refs rejects empty ref lists with an explicit ValueError naming
  the key and likely cause, instead of a cryptic np.stack([]) crash.
- num_resims converts the numpy status-array reduction to a native int at
  the boundary; np.int64 previously leaked through Ray into torch calls.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
buffer/n_training counted only SampleStatus.TRAINING, but the training
dataloader draws from TRAINING + DISFAVOURED (BufferView.cached_loader)
and rotate_sample_buffer floors that *sum* at min_samples.  The metric
therefore read below min_samples as soon as any sample was disfavoured,
even though those samples are still trained on until they are tombstoned.

Report TRAINING | DISFAVOURED in both append() and append_refs(), and
keep n_disfavoured as the eviction-pending subset so the split is still
visible.  Logging only; no change to the lifecycle or to what is trained on.

Extracted from e712a3e on plan/gaussianized-flow-matching.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The main launch loop paces itself with time.sleep(simulate_interval), but
every iteration also sat in ray.wait(..., timeout=1) first, so the actual
simulation cadence was simulate_interval + 1s.  With the common
simulate_interval=1 that halved the simulation rate relative to the
configured value.

Drop the poll timeout to 0.05s so simulate_interval means what it says.

BEHAVIOUR CHANGE: existing configs will simulate faster than before --
roughly 2x at simulate_interval=1, converging to no change as
simulate_interval grows.  Configs tuned against the old effective cadence
may need simulate_interval raised by ~1s to match.

Note that during a graceful stop the sleep is skipped, so the loop now
polls at 50ms instead of 1s while waiting for training to wind down.

Extracted from 6f4c6bb on plan/gaussianized-flow-matching.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
append_refs() was awaited synchronously, so the launch loop stalled on
the dataset manager actor (ray.put of every new sample, plus
rotate_sample_buffer) before it could start the next simulation round.

Keep one append in flight instead: block on the previous handle, fire the
new one, and flush whatever is outstanding when the loop exits so nothing
is lost at shutdown.

Consequence: num_resims() is now queried before the previous append has
landed, so the buffer can transiently exceed max_samples by up to one
simulate_count batch.  rotate_sample_buffer() tombstones the excess on the
next append, so the steady state is unchanged.  Exceptions raised inside
append_refs surface one iteration later than before.

Extracted from 6f4c6bb on plan/gaussianized-flow-matching.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 10.31%. Comparing base (d3c33ba) to head (0632b05).

Files with missing lines Patch % Lines
src/falcon/core/deployed_graph.py 0.00% 10 Missing ⚠️
src/falcon/core/raystore.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
- Coverage   10.33%   10.31%   -0.03%     
==========================================
  Files          30       30              
  Lines        3927     3936       +9     
==========================================
  Hits          406      406              
- Misses       3521     3530       +9     
Flag Coverage Δ
unit 10.31% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cweniger
cweniger merged commit b03b2b7 into main Aug 8, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant