Skip to content

Conversation

@farook-edev
Copy link
Contributor

No description provided.

@farook-edev farook-edev requested review from a team and anhappdev as code owners November 11, 2025 05:15
@github-actions
Copy link

github-actions bot commented Nov 11, 2025

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@farook-edev farook-edev marked this pull request as draft November 11, 2025 05:29
@farook-edev farook-edev marked this pull request as ready for review November 16, 2025 08:27
Copy link
Contributor

@freedomtan freedomtan left a comment

Choose a reason for hiding this comment

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

LGTM.

@freedomtan
Copy link
Contributor

freedomtan commented Nov 18, 2025

  • Should we add the unit for each numbers explicitly on the screen or we can leave them to tooltips when scores are clicked.
    • we are more adding the unit explicitly on the screen, let's think where to add the units. e.g., make "task name" clickable, and add the unit the i column. or make the text clickable.
  • we should change the history screen, too

@farook-edev
Copy link
Contributor Author

I've added the units next to the result values. As for the 🛈 icon, I was going to make multiple versions and see which one looked best, but then I noticed that the 'make the icon a clickable button and put the info there' idea has already been implemented and approved for the config screen as per #1028. So I opted to replicate the same UI in the result screen.

I also edited the history screen to have both QPS average for non token benchmarks and TPS average for token benchmarks. In addition to adding T/sec or Q/sec for each benchmark in the table below the summary.

@sonarqubecloud
Copy link

@farook-edev farook-edev marked this pull request as draft November 25, 2025 05:43
@freedomtan
Copy link
Contributor

freedomtan commented Nov 25, 2025

please add the time to first token to the "Result details"
let the time to first token time be the first enty.

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.

3 participants