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

Add val option for mtv requiring sim tracks matched to seeds #229

Merged
merged 1 commit into from
Jul 19, 2019

Conversation

areinsvo
Copy link
Collaborator

Using --mtv-require-seeds, you can now get MTV-like validation but with the additional requirements that the sim tracks are matched to a seed. All of the benchmark and validation plots are here, including the SIMVAL for the extra option and the normal MTV-like validation.
http://areinsvo.web.cern.ch/areinsvo/MkFit/Benchmarks/July2019_AddValOpt/

@kmcdermo
Copy link
Collaborator

@kmcdermo
Copy link
Collaborator

Benchmarks + validation look good, code looks good as well. +1 from me

@srlantz
Copy link
Collaborator

srlantz commented Jul 17, 2019

Oof, this is not so nice: http://areinsvo.web.cern.ch/areinsvo/MkFit/Benchmarks/July2019_AddValOpt/SIMVAL_MTV-require-seeds/SKL-SP_CMSSW_TTbar_PU70_eff_eta_build_pt0p0_SIMVAL.png

thanks for making this PR!

But (as I'm sure you're aware, @kmcdermo) at least it's obvious that the "not so nice" part is coming almost entirely from low pT, so it's a known issue: http://areinsvo.web.cern.ch/areinsvo/MkFit/Benchmarks/July2019_AddValOpt/SIMVAL_MTV-require-seeds/SKL-SP_CMSSW_TTbar_PU70_eff_eta_build_pt2p0_SIMVAL.png

@kmcdermo
Copy link
Collaborator

kmcdermo commented Jul 17, 2019 via email

@srlantz
Copy link
Collaborator

srlantz commented Jul 17, 2019

@areinsvo Should we assume your benchmarks were built and run with the new default compiler on the UCSD machines, which would be icc 19.0.4? If so, it would be interesting to compare them with the benchmarks that @osschar recently ran based on that compiler (though I don't know where he posted the results)

@areinsvo
Copy link
Collaborator Author

@srlantz I ran the benchmarks last night, so I think the answer is yes, but @osschar will have to comment. You're right that we should compare them to his new benchmarks and make sure the results agree.

@osschar
Copy link
Collaborator

osschar commented Jul 18, 2019

yes, this must have been with 19.0.4 ... I just posted the devel benchmark here:
http://xrd-cache-1.t2.ucsd.edu/matevz/PKF/1000-icc-14.0.4/

@osschar
Copy link
Collaborator

osschar commented Jul 18, 2019

blak, typo in major version, moved it to proper name:
http://xrd-cache-1.t2.ucsd.edu/matevz/PKF/1000-icc-19.0.4/

@slava77
Copy link
Collaborator

slava77 commented Jul 18, 2019

blak, typo in major version, moved it to proper name:
http://xrd-cache-1.t2.ucsd.edu/matevz/PKF/1000-icc-19.0.4/

is there a reference for this as well?

@osschar
Copy link
Collaborator

osschar commented Jul 18, 2019

well, the plots from the last PR ... there was no other change than compiler version.

@osschar
Copy link
Collaborator

osschar commented Jul 18, 2019

i didn't change anything -- that was sort of the point of this benchmark :)

@slava77
Copy link
Collaborator

slava77 commented Jul 18, 2019

i didn't change anything -- that was sort of the point of this benchmark :)

Is there a way to rerun the baseline/reference now?

@cerati
Copy link
Collaborator

cerati commented Jul 18, 2019 via email

@dan131riley
Copy link
Collaborator

TBB went from the 2019.0 release to 2019.6. I don't see anything obvious that would improve our scaling so much.

@srlantz
Copy link
Collaborator

srlantz commented Jul 18, 2019

@slava77 - The older icc 19.0.0 is still present on phi3. Even though it's not the one you get when you run the default compilervars.sh, you can choose it preferentially over the newer 19.0.4 by issuing the following command:

source /opt/intel/compilers_and_libraries_2019.0.117/linux/bin/compilervars.sh intel64

The interesting aspect of the speedup you noticed is that the single-thread timing result is unchanged, but all other points on the curve apparently improved by a factor varying from 1.1x faster (at 2 threads) to 1.6x faster (at 64 threads).

@slava77
Copy link
Collaborator

slava77 commented Jul 18, 2019

going back to #228 http://xrd-cache-1.t2.ucsd.edu/matevz/PKF/208-Optional-HitSorting-Prefetch-Cleanup/ , it looks like the results there are very similar to the "baseline" from #227 already shared earlier.

@kmcdermo
Copy link
Collaborator

Given the discussion that the speedup does not appear anomalous, I propose to merge these changes. If we want to rerun the benchmarks with the old vs. new compiler, I can do that as part of Issue #230.

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

Successfully merging this pull request may close these issues.

7 participants