Feat/csv cpu gpu pct headers - #23
Merged
Merged
Conversation
The merged CSV mixed unqualified CPU names (target_ms_*, target_pct_*) with explicit GPU names (target_gpu_ms_*), and had no GPU equivalent of the CPU '% of frame' figure. Make it symmetric: rename target_ms_*->target_cpu_ms_*, target_pct_*->target_cpu_pct_*, the per-row target_pct_of_frame->target_cpu_pct_of_frame; and add target_gpu_pct_{mean,min,max} stat lines + a per-row target_gpu_pct_of_frame column (target_gpu_us / frame_interval_us * 100, blank when the GPU sample or the interval is missing).
Applied to both producers (merge.cpp + analyze.py) so the byte-equivalence contract holds, plus the test_merge.cpp golden tests, the test_analyze.py assertions, and the README format docs. analyze.py validated end-to-end (19/19 pytest) on a fixture: GPU pct computes and blanks on the last frame per thread.
BREAKING: merged-CSV column/stat names changed; update any external spreadsheet/script keyed on target_ms_*, target_pct_*, or the target_pct_of_frame column.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e_count A frame count is a cardinality, not a target cost, so the target_ prefix (reserved for target_gpu_ms_* / target_gpu_pct_*) does not belong on it. This also aligns the CSV header with the internal MergeStats field, already named gpu_frame_count. frame_count (the overall matched count) stays bare for the same reason -- header counts are now frame_count + gpu_frame_count. Updated both producers, the golden tests, and the README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…change The CPU-rename + GPU-pct + gpu_frame_count commits widened the merged CSV to fourteen # lines (7 CPU + 7 GPU) and nine columns ending in target_gpu_pct_of_frame, but several contracts/comments/examples still said eleven lines / eight (or seven) columns / target_gpu_us-trailing / '4 GPU lines', and the README GPU-pct example count ignored the last-frame-per-thread exclusion it documents. Self-review fixes: - merge.h: WriteMergedCsv contract (fourteen lines, trailing target_gpu_pct_of_frame) + kExpectedColumnHeader '9-column' note. - README: schema prose (fourteen / seven+seven) + GPU-pct example count 1839->1838. - analyze.py / test_analyze.py / test_merge.cpp: 'nine columns' / '7 GPU lines'. Comment/doc only; no behavior change (19/19 pytest green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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.
No description provided.