[tuner] Add BaselineResultHandler class #789

bangtianliu · 2025-01-08T19:36:25Z

This PR is relevant to address the task in: #783.

This PR adds BaselineResultHandler class to handle the baseline results and related functionality.

Add reusable benchmarking helper functions benchmark_baseline and benchmark_candidates to run baseline and candidates.
Add BaselineResultHandler class to handle both baseline and candidate benchmark results including performance regression detection, speedup calculation and candidate selection.
Add corresponding tests.

Max191

Overall looks good. Just a few nits about the logging and code reuse. Could you also add a more informative PR description (essentially explain the reasoning for benchmarking baselines twice like in the linked issue).

tuner/tuner/libtuner.py

Max191

Looks good, thanks! Just one nit.

tuner/tuner/libtuner.py

kuhar · 2025-01-09T15:54:42Z

Ideally, these corner cases should be covered by unit tests.

bangtianliu · 2025-01-09T20:43:54Z

WIP: need to rebase on top of #799, and continue to work.

kuhar

Can you edit the PR description and explain what error conditions you considered and how they are handled?

tuner/tuner/candidate_gen.py

tuner/tuner/libtuner.py

tuner/tuner/libtuner_test.py

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

tuner/tuner/libtuner.py

tuner/tuner/libtuner_test.py

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

kuhar

LGTM % one suggestion.

Thanks for all the changes, the code looks much more robust now.

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

tuner/tuner/libtuner.py

Signed-off-by: Bangtian Liu <[email protected]>

bangtianliu requested review from kuhar and Max191 January 8, 2025 19:36

bangtianliu force-pushed the benchmark_baseline branch 2 times, most recently from bd88a9c to 46d6a87 Compare January 8, 2025 19:40

Max191 reviewed Jan 8, 2025

View reviewed changes

tuner/tuner/libtuner.py Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

bangtianliu force-pushed the benchmark_baseline branch from 46d6a87 to 222f783 Compare January 9, 2025 00:35

bangtianliu requested a review from Max191 January 9, 2025 00:37

kuhar requested changes Jan 9, 2025

View reviewed changes

bangtianliu force-pushed the benchmark_baseline branch from 222f783 to 4b2165c Compare January 9, 2025 02:47

bangtianliu requested a review from kuhar January 9, 2025 02:54

Max191 approved these changes Jan 9, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

kuhar reviewed Jan 9, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

bangtianliu force-pushed the benchmark_baseline branch from 9593df8 to f139ee1 Compare January 9, 2025 23:17

bangtianliu requested a review from kuhar January 9, 2025 23:17

bangtianliu force-pushed the benchmark_baseline branch 3 times, most recently from 30ab7e4 to 6c88791 Compare January 9, 2025 23:22

kuhar reviewed Jan 10, 2025

View reviewed changes

bangtianliu force-pushed the benchmark_baseline branch 5 times, most recently from 6966557 to 4c52419 Compare January 10, 2025 04:51

bangtianliu changed the title ~~[tuner] Run baseline benchmarks first and check regression~~ [tuner] Run baseline benchmarks first with reusable helper function and regression detection Jan 10, 2025

bangtianliu requested a review from kuhar January 10, 2025 15:00

kuhar reviewed Jan 13, 2025

View reviewed changes

bangtianliu force-pushed the benchmark_baseline branch from 707455f to f0b4640 Compare January 13, 2025 17:16

bangtianliu requested a review from kuhar January 13, 2025 17:17

bangtianliu added 5 commits January 17, 2025 12:50

improve the baseline result handler

e454ca6

Signed-off-by: Bangtian Liu <[email protected]>

add more functions to handler class and format the tests

912e1b1

Signed-off-by: Bangtian Liu <[email protected]>

clean the code agin due to using handler class

3dbba8b

Signed-off-by: Bangtian Liu <[email protected]>

update the handler class

3d8fe4c

Signed-off-by: Bangtian Liu <[email protected]>

address reviewer comments and format code

4ca084b

Signed-off-by: Bangtian Liu <[email protected]>

bangtianliu force-pushed the benchmark_baseline branch from 44b7e92 to 4ca084b Compare January 17, 2025 18:51

bangtianliu requested a review from kuhar January 17, 2025 18:51

bangtianliu changed the title ~~[tuner] Run baseline benchmarks first with reusable helper function and regression detection~~ [tuner] adds BaselineResultHandler class to handle the baseline results and related functionality. Jan 17, 2025

bangtianliu changed the title ~~[tuner] adds BaselineResultHandler class to handle the baseline results and related functionality.~~ [tuner] Adds BaselineResultHandler class Jan 17, 2025

bangtianliu changed the title ~~[tuner] Adds BaselineResultHandler class~~ [tuner] Add BaselineResultHandler class Jan 17, 2025

kuhar reviewed Jan 17, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

change the function get_top_candidates

f188afe

Signed-off-by: Bangtian Liu <[email protected]>

bangtianliu requested a review from kuhar January 17, 2025 21:51

kuhar reviewed Jan 17, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner_test.py Outdated Show resolved Hide resolved

kuhar reviewed Jan 17, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

bangtianliu force-pushed the benchmark_baseline branch from 859cfc6 to f453016 Compare January 17, 2025 22:35

bangtianliu requested a review from kuhar January 17, 2025 22:35

update the code comments

180c435

Signed-off-by: Bangtian Liu <[email protected]>

bangtianliu force-pushed the benchmark_baseline branch from f453016 to 180c435 Compare January 17, 2025 22:37

kuhar reviewed Jan 18, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

merge two functions to calculate the speedup internally

2182ff4

Signed-off-by: Bangtian Liu <[email protected]>

kuhar approved these changes Jan 19, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

simplify the code

4d96e98

Signed-off-by: Bangtian Liu <[email protected]>

kuhar requested changes Jan 20, 2025

View reviewed changes

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

tuner/tuner/libtuner.py Outdated Show resolved Hide resolved

bangtianliu force-pushed the benchmark_baseline branch from b4536bb to b5467f2 Compare January 20, 2025 15:59

avoid using union

05e84ca

Signed-off-by: Bangtian Liu <[email protected]>

bangtianliu force-pushed the benchmark_baseline branch from b5467f2 to 05e84ca Compare January 20, 2025 16:00

bangtianliu requested a review from kuhar January 20, 2025 16:23

kuhar approved these changes Jan 20, 2025

View reviewed changes

bangtianliu merged commit 26bb250 into nod-ai:main Jan 20, 2025
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tuner] Add BaselineResultHandler class #789

[tuner] Add BaselineResultHandler class #789

bangtianliu commented Jan 8, 2025 •

edited

Loading

Max191 left a comment

Max191 left a comment •

edited

Loading

kuhar commented Jan 9, 2025

bangtianliu commented Jan 9, 2025

kuhar left a comment

kuhar left a comment

[tuner] Add BaselineResultHandler class #789

[tuner] Add BaselineResultHandler class #789

Conversation

bangtianliu commented Jan 8, 2025 • edited Loading

Max191 left a comment

Choose a reason for hiding this comment

Max191 left a comment • edited Loading

Choose a reason for hiding this comment

kuhar commented Jan 9, 2025

bangtianliu commented Jan 9, 2025

kuhar left a comment

Choose a reason for hiding this comment

kuhar left a comment

Choose a reason for hiding this comment

bangtianliu commented Jan 8, 2025 •

edited

Loading

Max191 left a comment •

edited

Loading