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

Refactor Superset model benchmarking tools to use Pydantic classes and save one json #16790

Merged
merged 10 commits into from
Jan 17, 2025

Conversation

skhorasganiTT
Copy link
Contributor

@skhorasganiTT skhorasganiTT commented Jan 15, 2025

Ticket

#15435

Problem description

  • The current benchmark infra for uploading data to Superset is using the deprecated method of generating 3 separate CSV files for run/measurement/environment data.
  • The Llama3 t3k benchmark data was not being uploaded by the t3k demo tests

What's changed

  • Refactored the model benchmarking tools to generate a single json file which is partially filled with run/measurement data during model tests, and afterwards completed with environment data by the CI test infra
  • Refactored the benchmark data to be stored using the Pydantic classes implemented by the data science team so that data types are validated prior to json creation
  • Updated the benchmark environment job in the Produce data for external analysis workflow to test completing a partial benchmark json
  • Minor var cleanup in demo tests, modifying 'csv' to 'json'
  • Enabled uploading of benchmark data for the llama3 T3K demo tests

Checklist

  • Post commit CI passes
  • Blackhole Post commit (if applicable)
  • Model regression CI testing passes (if applicable)
  • Device performance regression CI testing passes (if applicable)
  • (For models and ops writers) Full new models tests passes
  • New/Existing tests provide coverage for changes

…l json and add data uploading for t3k llama tests

Signed-off-by: Salar Hosseini <[email protected]>
Signed-off-by: Salar Hosseini <[email protected]>
Signed-off-by: Salar Hosseini <[email protected]>
Signed-off-by: Salar Hosseini <[email protected]>
@skhorasganiTT
Copy link
Contributor Author

skhorasganiTT commented Jan 15, 2025

Produce data for external analysis test: https://github.com/tenstorrent/tt-metal/actions/runs/12818671201
Single-card demo tests: https://github.com/tenstorrent/tt-metal/actions/runs/12815226849
T3K demo tests: https://github.com/tenstorrent/tt-metal/actions/runs/12815926839
All post-commit tests: https://github.com/tenstorrent/tt-metal/actions/runs/12815232660

Copy link
Contributor

@mtairum mtairum left a comment

Choose a reason for hiding this comment

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

Looks good from the llama3 demo side.

I'll be updating my local changes to use the new pydantic as well 👍

Copy link
Collaborator

@tt-rkim tt-rkim left a comment

Choose a reason for hiding this comment

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

I propose a decently big change in benchmarking_utils.py

Rest of the code is ok

models/perf/benchmarking_utils.py Outdated Show resolved Hide resolved
models/perf/benchmarking_utils.py Show resolved Hide resolved
@skhorasganiTT skhorasganiTT merged commit af79262 into main Jan 17, 2025
17 of 23 checks passed
@skhorasganiTT skhorasganiTT deleted the skhorasgani/superset_json branch January 17, 2025 17:14
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.

5 participants