Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Low latency checks to submission checker and report low latency #2017

Merged
merged 8 commits into from
Jan 21, 2025

Conversation

pgmpablo157321
Copy link
Contributor

  • Add llama2 low latencies to configuration file
  • Add low latency submission checks
  • Report low latency result in a different column

@pgmpablo157321 pgmpablo157321 requested a review from a team as a code owner January 6, 2025 19:37
Copy link
Contributor

github-actions bot commented Jan 6, 2025

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

@pgmpablo157321 pgmpablo157321 force-pushed the low_latency_submission_checker branch 2 times, most recently from 43f9059 to 7fa675c Compare January 7, 2025 23:36
@mrmhodak
Copy link
Contributor

@pgmpablo157321 : In the README, you show an example of running Offline in the Interactive mode. That can be confusing, because Offline is the same and only Server changes.

I would suggest that the example would be for server and say that for the Offline run nothing changes with the new flags.

# settings.FromConfig(args.mlperf_conf, "llama2-70b", args.scenario)
settings.FromConfig(args.user_conf, "llama2-70b", args.scenario)
# settings.FromConfig(args.mlperf_conf, args.lg_model_name, args.scenario)
settings.FromConfig(args.user_conf, args.lg_model_name, args.scenario)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we still comment out the llama2-70b-interactive if we are loading from the user config here?

Copy link
Contributor

@nvzhihanj nvzhihanj Jan 13, 2025

Choose a reason for hiding this comment

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

Actually, do we still need the user.conf change if the mlperf.conf already has the new latency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmmm there might be other settings that submitters could modify. So, leaving the user.conf there might be helpful

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right, nvm

@pgmpablo157321 pgmpablo157321 force-pushed the low_latency_submission_checker branch from ea45000 to 9d322cb Compare January 16, 2025 18:41
@arjunsuresh
Copy link
Contributor

@pgmpablo157321 the github action for submission checker is testing the submissions from v4.1 with the v5.0 submission checker code. That's why they are failing. Please ignore it.

I'll add a test submission results repo soon with v5.0 results.

@nvzhihanj
Copy link
Contributor

We have tested the PR and we can merge it. We will raise issues if we find any

@mrmhodak mrmhodak merged commit ba71b21 into master Jan 21, 2025
25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants