Skip to content

fix(sa-bench): fix rollup key mapping for p99 latencies, token counts, and ISL/OSL - #241

Open
arpera wants to merge 5 commits into
NVIDIA:mainfrom
arpera:fix-sa-bench-rollup
Open

fix(sa-bench): fix rollup key mapping for p99 latencies, token counts, and ISL/OSL#241
arpera wants to merge 5 commits into
NVIDIA:mainfrom
arpera:fix-sa-bench-rollup

Conversation

@arpera

@arpera arpera commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Rollup key mapping is wrong for sa-bench. Some of the fields were affected by this (for example, ISL/OSL) resulting in being None in the final json statistics file.

Testing Done

  • Manual testing of sa-bench after applying this patch showed no None fields in json statistics file.
  • make check -- no issues found

…, and ISL/OSL

Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>

valid_low_concurrency = {
"model_id": "GLM-5-FP8",
"random_input_len": 8192,

@KaunilD KaunilD Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hold up. they've removed isl and osl from the spec ? this a load bearing metric for fixed isl osl workloads. the conforms with agentic workloads where total tokens matter more. let me confirm from the latest ci run from inferencex.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kaunil, probably I misunderstand something but I did this PR only targeting sa-bench implementation from srt-slurm (not InferenceX's sa-bench). I suppose InferenceX has it's own implementation of sa-bench and this implementation differs from srt-slurm's one (particularly InferenceX's one has isl/osl field in results json file, but srt-slurm's one does not have them). If I understand right then srt-slurm's implementation of sa-bench does not have isl filed in json file. So, my point is that in this PR I am attempt to fix srt-slurm's sa-bench results parsing not InferenceX's sa-bench. Could you tell me please what I should do then?

Comment thread src/srtctl/benchmarks/scripts/sa-bench/rollup.py Outdated
Comment thread src/srtctl/benchmarks/scripts/sa-bench/rollup.py Outdated
Comment thread src/srtctl/benchmarks/scripts/sa-bench/rollup.py Outdated
Comment thread src/srtctl/benchmarks/scripts/sa-bench/rollup.py Outdated
arpera and others added 4 commits July 10, 2026 21:23
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants