core: simulation scheduling fixes + buffer accounting - #117
Merged
Conversation
- _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 Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
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 ofd46aac4)Crash fix. When
n_samples < num_actors,MultiplexNodeWrapper._multiplexed_callhands some actors an empty index slice. That reaches_resolve_refs, where an empty ref list falls past the broadcast check intonp.stack([])and dies with an unhelpfulneed at least one array to stack.Idle actors are now skipped, and
_resolve_refsgained an explicitValueErroras a backstop that names the actual cause. Also convertsnum_resims()fromnp.int64to a nativeint.No behaviour change on the normal path.
2.
core: report the actual trainable pool in buffer/n_training(extracted frome712a3e)Metric fix, logging only.
buffer/n_trainingcounted onlySampleStatus.TRAINING, butBufferView.cached_loaderdraws from[TRAINING, DISFAVOURED]androtate_sample_bufferfloors that sum atmin_samples. The metric therefore read belowmin_samplesas soon as anything was disfavoured, even though those samples are still being trained on until tombstoned.n_disfavouredis unchanged and still shows the eviction-pending subset, so the split stays visible.3.
core: stop the ray.wait timeout from inflating simulate_interval(from6f4c6bb)The launch loop paces itself with
time.sleep(simulate_interval), but each iteration first sat inray.wait(..., timeout=1). The real cadence wassimulate_interval + 1s. At the commonsimulate_interval=1that halved the simulation rate relative to the configured value.The poll timeout drops to 0.05s so
simulate_intervalmeans what it says. Existing configs will simulate roughly 2x faster atsimulate_interval=1, converging to no change assimulate_intervalgrows. Anything tuned against the old effective cadence wantssimulate_intervalraised 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(from6f4c6bb)append_refs()was awaited synchronously, stalling the launch loop on the dataset-manager actor (aray.putper sample, plusrotate_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 exceedmax_samplesby up to onesimulate_countbatch.rotate_sample_buffer()tombstones the excess on the next append, so the steady state is unchanged. Exceptions fromappend_refsnow 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 torchfails in my environment (libcudnn.so.9: cannot open shared object file), which also breakspytest testscollection onmain, so it is not something this branch introduces. Verification so far is static review pluspy_compile.tests/test_examples_smoke.pyshould be run on a GPU node before merge, mainly to exercise commit 3's timing change end to end.🤖 Generated with Claude Code