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

Enable DYNAMIC_CPU_ARCH in ONNXRuntime to use up to AVX2 #6855

Merged
merged 1 commit into from Apr 29, 2021

Conversation

hqucms
Copy link
Contributor

@hqucms hqucms commented Apr 28, 2021

As discussed in cms-sw/cmssw#32883 (comment), we would like to test the dynamic arch option in ONNXRuntime. The MLAS_DYNAMIC_CPU_ARCH is changed from 0 (no dynamic arch) to 2 (dynamic arch, use up to AVX2).

Enable dynamic_arch in ONNXRuntime to use up to AVX2
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @hqucms (Huilin Qu) for branch IB/CMSSW_12_0_X/master.

@cmsbuild, @smuzaffar, @mrodozov can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Apr 28, 2021

@cmsbuild please test

@slava77
Copy link
Contributor

slava77 commented Apr 28, 2021

abort test

@slava77
Copy link
Contributor

slava77 commented Apr 28, 2021

enable profiling

@slava77
Copy link
Contributor

slava77 commented Apr 28, 2021

@cmsbuild please test

@smuzaffar
Copy link
Contributor

@hqucms , have you tested locally by setting MLAS_DYNAMIC_CPU_ARCH=2 and if that works? Do we need any build option for onnxruntime itself to build/support avx?

@hqucms
Copy link
Contributor Author

hqucms commented Apr 28, 2021

@hqucms , have you tested locally by setting MLAS_DYNAMIC_CPU_ARCH=2 and if that works? Do we need any build option for onnxruntime itself to build/support avx?

@smuzaffar Yes -- this is a runtime flag and onnxruntime does not need to be re-built. Basically onnxruntime detects at runtime the cpu flags and will load specific kernels if avx/avx2/avx512 are available (https://github.com/cms-externals/onnxruntime/blob/cms/v1.7.2/onnxruntime/core/mlas/lib/platform.cpp#L178).

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-85141b/14674/summary.html
COMMIT: fae78e6
CMSSW: CMSSW_12_0_X_2021-04-28-1100/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/6855/14674/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2877605
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2877581
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2021

I can't find the timing piecharts in the profiling results.

@gartung
are the jobs making the piecharts and I'm just missing the link or is it not made?

@gartung
Copy link
Member

gartung commented Apr 29, 2021

Click on "see logs" links next to profiling results.

@gartung
Copy link
Member

gartung commented Apr 29, 2021

I thought I have FasterTimer service enabled for PR profiling. I will have to check the PR profiling job for failures.

@gartung
Copy link
Member

gartung commented Apr 29, 2021

The PR profiling job was missing a parameter/environment variable which is checked by the profiling script before using the FastTimeService python wrapper script that enables the json output.
https://github.com/cms-cmpwg/profiling/blob/a3d2d8ad080ea5a52bc57a0ca05f3197280b5888/Gen_tool/runall.sh#L48
I added the parameter and restarted the PR profiling job.

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2021

I added the parameter and restarted the PR profiling job.

Thank you.

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2021

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

@slava77,@perrotta,@jpata you have been requested to review this Pull request/Issue and eventually sign? Thanks

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2021

+reconstruction

based on #6855 (comment)

@smuzaffar
Copy link
Contributor

+externals

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_12_0_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2021

@gartung @smuzaffar
is there a way to control which workflow is used in profiling?
23434.21 takes much longer to run and in this case 11834.21 was enough.

@gartung
Copy link
Member

gartung commented Apr 29, 2021

Right now the workflows are read from a file
https://github.com/cms-sw/cms-bot/blob/0f346529f6fa8131c9ad4de6fac2121a16b351e2/pr_testing/run-pr-profiling.sh#L4
I can change that the allow override from the environment which can be controlled by a job parameter. I think that is something that can be set with a comment.

@smuzaffar
Copy link
Contributor

sounds good @gartung . You should also update https://github.com/cms-sw/cms-bot/blob/master/process_pr.py#L57 to support workflow_profiling

@gartung
Copy link
Member

gartung commented Apr 29, 2021

So that maps to the variable MATRIX_EXTRAS?

@gartung
Copy link
Member

gartung commented Apr 29, 2021

I need to add PROFILING here so MATRIX_EXTRAS_PROFILING is defined
https://github.com/cms-sw/cms-bot/blob/a1194080497a13d952360c64a2e60643f90108fc/pr-schedule-tests#L78

@gartung
Copy link
Member

gartung commented Apr 29, 2021

test parameters:
workflows_profiling=11834.21

@smuzaffar
Copy link
Contributor

I do not think that this will work without merging the cms-bot PR. Strange that bot did not reject your workflows_profiling=11834.21 parameter ...

@gartung
Copy link
Member

gartung commented Apr 29, 2021

I was setting up this PR to test the cms-bot PR.

@gartung
Copy link
Member

gartung commented Apr 29, 2021

please test with cms-sw/cms-bot#1545

@gartung
Copy link
Member

gartung commented Apr 29, 2021

abort

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants