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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

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

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

@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.

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.

4 participants