Instrument the rollout with nested timers (fresh branch of #1198) - #1238
Draft
alexmillane wants to merge 10 commits into
Draft
Instrument the rollout with nested timers (fresh branch of #1198)#1238alexmillane wants to merge 10 commits into
alexmillane wants to merge 10 commits into
Conversation
Timers now qualify their name with any enclosing timer, so nesting a "policy_inference" block inside a "step" block records under "step/policy_inference" and the registry keys spell out the call tree. Instruments the rollout loop with the breakdown that measurement showed dominates: the whole step, the policy inference inside it, and the env step, which splits further into the sim step below the recorders and the camera frame writes and encoder finalize beside it. The Experiment Runner prints the table and writes arena_experiment_timings.json, and the OSMO collect task merges every completed Run's timings into one Experiment-level file. Signed-off-by: alex <amillane@nvidia.com>
- Write the timer names inline at their call sites instead of exporting a constant per name, and repeat the literals in the tests. - Give the open-timer stack a threading.local subclass so each thread initializes its own, replacing the lazy getattr lookup. - Start qualified_name as None, since the constructor cannot know the enclosing timers, and resolve it on entry. - Import the timings filename with the other module-level imports. Signed-off-by: alex <amillane@nvidia.com>
- Shorten the comments on the step timer wrapper and drop the one on the camera frame timer. - Rename the camera frame timer to record_camera_frames. - Keep only the one-line docstring on _OpenTimerNames, with the threading.local note in the constructor. - Drop the separator-rejection and qualified_name tests. Signed-off-by: alex <amillane@nvidia.com>
Matches record_camera_frames beside it. Signed-off-by: alex <amillane@nvidia.com>
The OSMO workflow test pins substrings of the embedded build_experiment_output.py. That script gained the timings import and aggregation, but this assertion still expected the old single-name import line, so it had been failing on this branch. Signed-off-by: alex <amillane@nvidia.com>
The Experiment Runner wrote one timings file at the Experiment root covering every Run in the process, so a local multi-Run Experiment could not be split per environment. OSMO got a per-Run split for free by running each Run in its own process. - Clear the timer registry between Runs and write each Run's timings into its own output directory as it finishes. - Move the combining step into isaaclab_arena/evaluation/experiment_timings.py so the runner and the OSMO collect task share one implementation, and sort Runs by name so both produce the same file. - Print each Run's timing table as that Run finishes, rather than one table that only held the last Run's numbers. Signed-off-by: alex <amillane@nvidia.com>
- Cut the module and function docstrings in experiment_timings to a line each, and drop the comments on the rollout timers, the registry clear and the aggregation call. - Say what the combined timings file holds, including that each Run's entries carry the Run's name. - Hoist the completed Run names into a variable. Signed-off-by: alex <amillane@nvidia.com>
alexmillane
force-pushed
the
alex/feature/timing_3
branch
from
September 10, 2026 16:31
ad6ee4a to
4ee7784
Compare
experiment_timings imports utils.timer, which imports torch at module scope. Importing it at the top of experiment_runner pulled torch in before SimulationAppContext started AppLauncher, and Isaac Sim then crashed during startup in a fraction of subprocess-spawning tests. Import it inside main() alongside the other deferred evaluation imports. Signed-off-by: alex <amillane@nvidia.com>
timer.py imported torch at module scope, so every module importing Timer pulled torch in too. policy_runner and experiment_runner import Timer at module scope, and both run before SimulationAppContext starts AppLauncher, so torch initialized ahead of the Kit app. Isaac Sim then crashed during startup in roughly a third of subprocess-spawning test launches, against none on main. Import torch inside the two methods that use it, so importing Timer no longer drags torch in ahead of Isaac Sim. Signed-off-by: alex <amillane@nvidia.com>
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.
Summary
Same commits as #1198, on a fresh branch to rule out branch-scoped CI state.
Detailed description
Run tests with subprocesshas failed repeatedly onalex/feature/timing_2, always with an Isaac Sim startup crash (appState = 'startup') that then stalls the crash reporter for the full 900s subprocess timeout.mainand nine other branches (23 of 24 readable subprocess-job failures in the last week carry that signature), and 40 local runs of the most-affected test plus repeated local Phase 3 runs have not reproduced any crash.0517d499c, which already merges currentmain), so any difference in CI outcome is attributable to per-branch runner state — for example theactions/cachescope used by the Setup Isaac Sim kit cache step — rather than to the code.