unsortedOfflinePrimaryVertices4D speedup for 2023 timing workflows

[slava77]

related to discussion in #17610 leading to
#17610 (comment)

[cmsbuild]

A new Issue was created by @slava77 Slava Krutelyov.

@davidlange6, @Dr15Jones, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[slava77]

assign upgrade,reconstruction

@lgray

[cmsbuild]

New categories assigned: upgrade,reconstruction

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

[lgray]

Thanks. I'll try to get to it soon.

[Dr15Jones]

I do not believe vectorization will help this algorithm that much. vectorization could, at most, speed the algorithm up by a factor of 8, but from the measurements we were doing on the KNL the algorithm can be a 100x to 1000x slower than all other modules in the job. If you start with an event with very many primary vertices we saw it can takes 4 hours for the algorithm to converge.

[lgray]

Hi Chris, right now we are also dealing with an increase in the bulk timing (not just these outlier events, which we hope to eventually deal with). Vectorization is important here, and from the vectorization of the 1D version of the algorithm they gained a factor 2 or so, maybe we end up doing better. Either way the average time of the vertexing needs to be reduced at this point. Investigating the very long tails will come too.

…

-L

On Wed, Mar 8, 2017 at 5:23 PM, Chris Jones ***@***.***> wrote: I do not believe vectorization will help this algorithm that much. vectorization could, at most, speed the algorithm up by a factor of 8, but from the measurements we were doing on the KNL the algorithm can be a 100x to 1000x slower than all other modules in the job. If you start with an event with very many primary vertices we saw it can takes 4 hours for the algorithm to converge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17826 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBMOcOPKfqyo5qJZcg1RUA2WtiROwBMks5rjtYagaJpZM4MW-z_> .

[slava77]

@lgray
please check if you also need this fix
https://github.com/cms-sw/cmssw/pull/16764/files#diff-433c2174a94e2712d80a0b75bba3a49cL196
around https://github.com/lgray/cmssw/blob/8929924570c2ad59f9d21762df80786ed141e4d5/RecoVertex/PrimaryVertexProducer/src/DAClusterizerInZT.cc#L110

    // normalization
-    if (tks[i].zi>0){
+    if (tks[i].zi>epsilon){//e.g. constexpr double epsilon = 1.e-100

[slava77]

@lgray
what is the status of this issue?
Please "@" others who may be working on this.
Thank you.

[kpedro88]

+1
(fully) resolved by #20709