Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

extended test mode for benchmarks#352

Merged
Vyacheslav-Smirnov merged 5 commits intoIntelPython:masterfrom
samir-nasibli:benchamrk_ci_integration
Dec 2, 2019
Merged

extended test mode for benchmarks#352
Vyacheslav-Smirnov merged 5 commits intoIntelPython:masterfrom
samir-nasibli:benchamrk_ci_integration

Conversation

@samir-nasibli
Copy link
Copy Markdown
Contributor

  • extended test mode by adding benchmark mode

* extended test mode, added benchmark mode
@densmirn
Copy link
Copy Markdown
Contributor

Seems we need to add opportunity to run the performance tests under multiple threads by setting environment variables LOAD_PREV_RESULTS and NUMBA_NUM_THREADS before runs. Moreover we need to install extra Python modules openpyxl and xlrd to dump results to excel.

@Vyacheslav-Smirnov
Copy link
Copy Markdown
Contributor

Seems we need to add opportunity to run the performance tests under multiple threads by setting environment variables LOAD_PREV_RESULTS and NUMBA_NUM_THREADS before runs. Moreover we need to install extra Python modules openpyxl and xlrd to dump results to excel.

@densmirn ,
We expect to run benchmarks internally for now so these variables can be set there.
By the way - what is the purpose of LOAD_PREV_RESULTS and what is should point to?
openpyxl is already added to test env for benchmarks. Do we need to add xlrd also?

@dmitrii-zagornyi
Copy link
Copy Markdown
Contributor

dmitrii-zagornyi commented Nov 29, 2019

@densmirn @Vyacheslav-Smirnov
I strongly recommend to use csv files instead of excel files, adding dependencies for openpyxl, xlrd.

@Vyacheslav-Smirnov
Copy link
Copy Markdown
Contributor

@densmirn , Can we make benchmarks to output results in csv instead of excel?
This can move us from additional dependencies

@samir-nasibli
Copy link
Copy Markdown
Contributor Author

@densmirn I have the same question: what is the purpose of LOAD_PREV_RESULTS and what is should point to? It seems that currently excel files are overwriting without aggregation of recent results

@samir-nasibli
Copy link
Copy Markdown
Contributor Author

samir-nasibli commented Nov 29, 2019

@densmirn @Vyacheslav-Smirnov @dmitrii-zagornyi aggregating results in csv is much preferable in the task of integration benchmarks into CI, isn't it? also this would avoid additional dependencies

* update TestResults class's dump method by adding ability to save performance
  testing results into html files
@samir-nasibli samir-nasibli changed the title update test.py for benchmark-test extended test mode for benchmarks and added html dump Nov 29, 2019
@densmirn
Copy link
Copy Markdown
Contributor

Seems we need to add opportunity to run the performance tests under multiple threads by setting environment variables LOAD_PREV_RESULTS and NUMBA_NUM_THREADS before runs. Moreover we need to install extra Python modules openpyxl and xlrd to dump results to excel.

@densmirn ,
We expect to run benchmarks internally for now so these variables can be set there.
By the way - what is the purpose of LOAD_PREV_RESULTS and what is should point to?
openpyxl is already added to test env for benchmarks. Do we need to add xlrd also?

LOAD_PREV_RESULTS = 1 makes to load previous result. It can be useful in case when we run tests on different number of threads where results of each run are dumped to the file. E.g. after below runs we get table with all the results.

export LOAD_PREV_RESULTS=1
export NUMBA_THREADING_LAYER=omp (or tbb)
export NUMBA_NUM_THREADS=1
python -m sdc.runtests sdc.tests.tests_perf
export NUMBA_NUM_THREADS=2
python -m sdc.runtests sdc.tests.tests_perf 
export NUMBA_NUM_THREADS=4
python -m sdc.runtests sdc.tests.tests_perf

Maybe it makes sense to remove the file before the next series of run.

@densmirn
Copy link
Copy Markdown
Contributor

@densmirn @Vyacheslav-Smirnov @dmitrii-zagornyi aggregating results in csv is much preferable in the task of integration benchmarks into CI, isn't it? also this would avoid additional dependencies

In case if we want to get CSV results we need to wait #347.

Comment thread sdc/tests/tests_perf/test_perf_utils.py Outdated

class TestResults:
perf_results_xlsx = 'perf_results.xlsx'
perf_results_html = 'perf_results.html'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I vote for waiting for #347 to apply these changes over this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@densmirn , @PokhodenkoSA ,
Then I think result's dump to html should be implemented in #347
And in current PR we can revert changes with html and merge it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please resolve merge conflicts.

@Vyacheslav-Smirnov Vyacheslav-Smirnov changed the title extended test mode for benchmarks and added html dump extended test mode for benchmarks Dec 2, 2019
@Vyacheslav-Smirnov Vyacheslav-Smirnov merged commit d1af2d6 into IntelPython:master Dec 2, 2019
@samir-nasibli samir-nasibli deleted the benchamrk_ci_integration branch December 2, 2019 13:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants