Backfill total_s from Cam2V's model_step_wall_s for LingBot benchmark scenarios

[AmirF194]

Refs #564

Cam2V's session.py reports its per-step duration and rate as model_step_wall_s/chunk_fps, and _generated_fps_for_step/_generated_fps_summary only ever look at total_s (falling back to model_step_s), so LingBot's Cam2V scenarios never get a derived generated_fps, and harness.py's total_s highlights come up empty too.

I went with the "report total_s alongside the existing key" option rather than renaming: the runtime-metric-samples ingest path had no key-normalization step at all (unlike the stats-rows path, which already has _KEY_OVERRIDES for this), so I added a small alias step there that backfills total_s from model_step_wall_s when a step doesn't already report total_s. model_step_wall_s stays in the output. Once total_s is present, the existing fallback in _generated_fps_for_step/_generated_fps_summary derives generated_fps on its own, and harness.py's total_s_median/startup_step_total_s highlights and report.py's headline cards pick it up with no changes needed there.

This covers the LingBot scenarios dropping generated_fps and the headline cards, not the regression-gate part of the issue (the metric name lists that skip instead of failing). I couldn't find that gate's config in this repo, so if it lives in an internal tool this half may need a separate change there.

Added test_runtime_benchmark_stats_backfills_total_s_from_model_step_wall_s: feeds a model_step_wall_s/chunk_fps sample through records_from_stats_file and checks both total_s and generated_fps come out. The new test fails without the change. The existing runtime-metric-samples tests (group-by-step, warmup exclusion, the v2 sink one) still pass unchanged.

Couldn't run the full suite locally, this workspace needs CUDA/transformer-engine to build. Checked the change with ruff (check and format, both clean) and by loading tools/benchmarks/metrics.py directly with no other project dependencies, since the module itself only imports the standard library.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Confidence Score: 4/5

The PR is not yet safe to merge because the previously reported Cam2V post-processing throughput mismatch remains unresolved.

Findings

1.  Postprocessing Time Is Excluded ▶

Summary

• Applies aliases to per-step and summary runtime metrics.
• Adds a parser-level test covering total_s and generated_fps derivation from an injected Cam2V-shaped sample.

_{Reviews (4) · Last reviewed commit: "Backfill total_s from Cam2V's model_step..."}

[ZenAlexa]

The alias is applied inside records_from_stats_file(), after the runtime stats artifact has already been written. Cam2V's StepResult.metrics therefore continues to contain model_step_wall_s/chunk_fps and omits total_s, so the raw artifact and every direct consumer keep the non-canonical contract described in #564; the first acceptance criterion remains open. I added assert result.metrics["total_s"] == result.metrics["model_step_wall_s"] to test_model_loop_maps_wasd_to_shared_camera_input_and_metrics, and the PR head fails with KeyError: 'total_s'. Assigning total_s beside model_step_wall_s in Cam2VModelLoop.step() gives every consumer the canonical key. This parser alias can also remain for historical artifacts.

[AmirF194]

You're right, the alias only patched the ingest path, so StepResult.metrics itself never got total_s. Pushed 7f1270a: Cam2VModelLoop.step() now sets total_s alongside model_step_wall_s at the source, and the parser alias stays for historical artifacts.

Added your assertion to the existing WASD test: fails with KeyError: 'total_s' on the prior commit, passes now. Ran it CPU-only (torch cpu wheel, no GPU here) along with the rest of test_application.py and test_benchmark_harness.py, 59 passed, 1 skipped, ruff clean on both files.

[ZenAlexa]

Cam2V now emits total_s in the producer, and the parser preserves an explicit canonical value. The runtime-metrics and runtime-records tests pass at 7f1270a8 (59 passed, 1 skipped); restoring the previous producer reproduces KeyError: 'total_s' in the regression. The raw artifact, throughput summary, and report paths now share the metric.

[greptile-apps]

Postprocessing Time Is Excluded

When post-processing is enabled, StepResult.frame_count counts the presented post-processed frames, but total_s is set to the model-only duration and excludes the time measured by postprocess_step_wall_s. The benchmark then divides the post-processed frame count by model-only time, which overstates throughput; buffered output and final-tail frames can further distort individual steps. Use the complete loop duration for total_s, or pair the model-only duration with the model-generated frame count.

[AmirF194]

Rebased onto main. #607 (merged after this PR was opened) rewrote Cam2VModelLoop.step()'s metrics flow: StepResult now returns the raw state.pipeline.finalize() dict directly instead of the locally enriched metrics dict, so model_step_wall_s, chunk_fps and every other value this step used to add are no longer present in StepResult.metrics at all. The total_s line I pushed on top of the old metrics.update() call no longer reaches anything, so I dropped it and kept this PR scoped to the metrics.py key-normalization fix, which is independent of that flow and still verified working against current main.

That leaves the deeper gap open: Cam2V's runtime metric samples carry none of the per-step timing fields post-#607, not just total_s. Whether StepResult should go back to returning the enriched dict, or something else given #607's own note that it is temporary pending #603, is a call for someone closer to that migration. Happy to open a separate issue for it if that's useful.

[AmirF194]

This rebased cleanly onto the new metrics flow five days ago and hasn't had a maintainer look yet, flagging it in case it slipped by.