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

Integrate mass-benchmarking into the Files API #115

Closed
jeremyfowers opened this issue Feb 15, 2024 · 3 comments · Fixed by #200
Closed

Integrate mass-benchmarking into the Files API #115

jeremyfowers opened this issue Feb 15, 2024 · 3 comments · Fixed by #200
Labels
cleanup Cleaning up old/unused code and tech debt p1 Medium priority

Comments

@jeremyfowers
Copy link
Collaborator

As of #112, turnkey is able to benchmark build directories directly via the turnkey cache benchmark BUILD_NAME command.

#112 implements the functionality as a completely new code path. This is a technical debt because now there are 2 ways to do benchmarking (turnkey benchmark being the other), they may produce slightly different results (e.g., turnkey cache benchmark does a better job reporting hardware failures), and any changes to benchmarking policy now need to be repeated in two places.

Ideally, a single code path would be used for benchmarking anything, regardless of whether it is a python script, ONNX file, or build directory. However, at the time of #112 there were blockers that need to be resolved before integration can take place:

  1. Configurable verbosity in Analyzer Status #113
  2. Change --rebuild to --skip #114

Proposed solution:

  1. Merge Directly benchmark build directories #112
  2. Resolve the blockers as independent PRs
  3. Integrate the logic from Directly benchmark build directories #112 into the mainline benchmarking flow

cc @danielholanda

@jeremyfowers jeremyfowers added cleanup Cleaning up old/unused code and tech debt p1 Medium priority labels Feb 15, 2024
@jeremyfowers
Copy link
Collaborator Author

Open design question: once we introduce turnkey cache benchmark would we ever deprecate it in favor of turnkey benchmark cache/*?

Once the mass-benchmarking logic is mainlined into the Files API it would be trivial to enable turnkey benchmark cache/*, its just that would be a breaking change pretty soon after introducing turnkey cache benchmark.

@danielholanda
Copy link
Collaborator

Potential solution to avoid users taking dependency on the interface:

Print a warning when turnkey cache benchmark is used saying that it is an experimental/beta interface for the feature.

@jeremyfowers
Copy link
Collaborator Author

Potential solution to avoid users taking dependency on the interface:

Print a warning when turnkey cache benchmark is used saying that it is an experimental/beta interface for the feature.

Love this, I'm adding the notice to the turnkey cache benchmark PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Cleaning up old/unused code and tech debt p1 Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants