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 build_model() out of benchmark_model() #48

Merged
merged 18 commits into from
Dec 5, 2023

Conversation

jeremyfowers
Copy link
Collaborator

@jeremyfowers jeremyfowers commented Dec 1, 2023

Closes #50 by eliminating the benchmark_model() API
Closes #39 by collecting independent telemetry for builds and benchmarks
Closes #10 by creating the build marker on the proper line of code

Also fixes numerous documentation bugs that were referencing benchmark_model() when they should have been referencing build_model().

Demo

Build Killed

(tkml) jfowers@LAPTOP-5VK03G46:~/turnkeyml/models/transformers$ turnkey bert.py --process-isolation --timeout 10 --rebuild always

Times out during torch export.

in stats: build_status: running. Before, it would have said benchmark_status: running.

Shows in turnkey cache list and gets properly deleted in turnkey cache delete --all. Before, it would have been ignored.

turnkey cache list
Info: Builds available in cache ~/.cache/turnkey:
bert_transformers_bf722986
(tkml) jfowers@LAPTOP-5VK03G46:~/turnkeyml/models/transformers$ turnkey cache delete --all

Info: Deleted build: bert_transformers_bf722986

rebuild=never

After the killed build, run turnkey bert.py --rebuild never

This will throw Build intentionally skipped because rebuild=never as expected.

The stats say: build_status: running, which is the expected value (no mention of benchmark_status). Before, it would have said benchmark_status: failed which would have covered up the fact that the original build was killed.

 

Benchmark killed

Run turnkey bert.py --rebuild-always and then ctrl+c it after the build finishes.

In stats:

    benchmark_status: running
    build_status: successful

Before, it would have just said benchmark_status: running with no mention of build status.

@jeremyfowers jeremyfowers linked an issue Dec 1, 2023 that may be closed by this pull request
@jeremyfowers jeremyfowers changed the title 39 benchmarking status stat is ambiguous Refactor build_model() out of benchmark_model() Dec 1, 2023
@jeremyfowers jeremyfowers changed the base branch from main to canary December 2, 2023 20:43
@jeremyfowers jeremyfowers self-assigned this Dec 2, 2023
@jeremyfowers jeremyfowers force-pushed the 39-benchmarking_status-stat-is-ambiguous branch from f51621e to ac186be Compare December 2, 2023 20:48
@jeremyfowers jeremyfowers marked this pull request as ready for review December 2, 2023 21:39
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
@jeremyfowers jeremyfowers force-pushed the 39-benchmarking_status-stat-is-ambiguous branch from 2d14e0e to 5d95213 Compare December 4, 2023 14:55
@jeremyfowers
Copy link
Collaborator Author

https://test.pypi.org/project/turnkeyml/0.4.0/ available for testing

jeremyfowers and others added 2 commits December 4, 2023 11:17
Copy link
Collaborator

@danielholanda danielholanda left a comment

Choose a reason for hiding this comment

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

Tested locally on a few different scenarios.
Working as expected + reducing a ton of code.
Very happy!

src/turnkeyml/run/devices.py Outdated Show resolved Hide resolved
src/turnkeyml/run/devices.py Outdated Show resolved Hide resolved
src/turnkeyml/run/devices.py Outdated Show resolved Hide resolved
…:onnx/turnkeyml into 39-benchmarking_status-stat-is-ambiguous
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
@jeremyfowers jeremyfowers enabled auto-merge (squash) December 5, 2023 15:21
@jeremyfowers jeremyfowers merged commit 47248e1 into canary Dec 5, 2023
10 checks passed
@jeremyfowers jeremyfowers deleted the 39-benchmarking_status-stat-is-ambiguous branch December 5, 2023 15:30
jeremyfowers added a commit that referenced this pull request Dec 6, 2023
* Refactor build out of benchmarking

Signed-off-by: Jeremy Fowers <jeremy.fowers@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants