Add TimingShim for rollout loops positronic does not run - #494
Conversation
An external rollout loop (MolmoSpaces first) marks phases and outcomes on the shim, which writes the `timing.jsonl` records and recorded-episode stubs `positronic eval timing-report` joins — one `EpisodeTiming` schema owner, shared from `eval_timing`. `start_gpu_sampler` is promoted public there as the shared GPU sampler. Includes a host CPU/RAM sampler sidecar the reducer ignores. Requested-by: Vladimir Yakunin Ticket: Positronic-Robotics/internal#76 #refs
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edb980136e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Requested-by: Vladimir Yakunin Ticket: Positronic-Robotics/internal#76 #refs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb598749d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Append each `EpisodeTiming` to `timing.jsonl` as the episode seals instead of buffering until `close()`, so a pass killed mid-run keeps every completed rollout's timing. `run()` truncates the file up front; explicit `begin/finish` without `run()` truncates lazily on the first flush, and `close()` no longer rewrites the whole file (still returns the path, leaving an empty valid file for a zero-episode pass). Clear a stale `host_stats.log` in `run()`, symmetric with the `gpu_dmon.log` clear, so a prior pass's samples aren't mistaken for current telemetry with `sample_host=False`. Requested-by: Vladimir Yakunin Ticket: Positronic-Robotics/internal#76 #refs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cbecf8883
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Freeze `wall_s` and `finished_at` on the same side of the stub write in `finish_episode`, passing `finished_at` into `to_timing` instead of stamping it after the `_write_episode_dir` I/O. The reducer reconstructs an episode's start as `finished_at - wall_s`; capturing the two across the stub write skewed that start by the write duration on slow filesystems. The stub write is shim bookkeeping, not part of the rollout, so it now falls outside the episode window entirely. Requested-by: Vladimir Yakunin Ticket: Positronic-Robotics/internal#76 #refs
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f0145ee0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A preemption between the episode dir becoming visible and its last file landing left a half-written stub that `timing-report` read as a finished episode. Bracket the stub writes with LocalDataset's `.unfinished` marker (same contract as its writer) and clear it after the last write. Ticket: Positronic-Robotics/internal#73 #refs
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Closing unmerged — |
1 participant
What
Re-integrates the MolmoSpaces pilot's timing shim into positronic, stacked on #479 (
eval-timing), per the graduate-before-merge decision (internal#76): the durable half of nebius-competition PR #22 lands here before #22 merges.positronic/eval_timing_shim.py—TimingShim/Episode: an external rollout loop (MolmoSpaces is the first) marks phases (reset / policy / env step / record IO) and outcomes; the shim writes thetiming.jsonlrecords and the recorded-episode stubs (meta.json+static.json+ a timestamped parquet signal in the block/episode layout) thatpositronic eval timing-reportjoins byepisode_uid. Plus a/proc-based host CPU/RAM sampler sidecar (host_stats.log) the reducer deliberately ignores.EpisodeTimingand the dmon sampler as a wire contract; here they are imported fromeval_timing, whosestart_gpu_sampleris promoted public as the shared GPU sampler (only rename + a log-prefix neutralization touch Opt-in wall-clock telemetry forpositronic eval run+eval timing-report#479's file).positronic/tests/test_eval_timing_shim.py— ported from the pilot: schema/invariant round-trip through the realEpisodeTiming, join-layout, abort-discard, GPU/host sampler behavior, and an end-to-end reduce through the realpositronic eval timing-reportCLI in-repo (the pilot version shelled out to a hardcoded external worktree and skipped when absent — now it always runs).Changes vs. the pilot source, beyond the imports: PEP 723 header dropped, pyarrow is a hard import (core dependency here),
/proc-dependent tests skip cleanly off-Linux, and the pilot'srun_pilot.pytests stayed behind (experiment glue, moving toexperiments/002in #22's rework).Testing
pytest positronic/tests/ --no-cov: 21 passed, 4 skipped (the 9 new tests all pass, e2e reduce included).ruff check+formatclean. No references to the old private sampler name remain.🤖 Generated with Claude Code