remove unnecessary first pass on all bridge events in cudacpp helicity calculation from madevent (and improve timers)

[valassi]

This is a WIP PR to improve timers and cudacpp helicity computation in madevent #958 and #546

[valassi]

Hi @oliviermattelaer this is essentially ready for review. Can you please reviww?
(But I will still launch some manual tests on all processes)

I renamed it as "remove unnecessary first pass on all bridge events in cudacpp helicity calculation from madevent (and improve timers)". Essentially what this does is

• I added a boolean parameter "goodhelonly" to the fortran-cpp interface of the bridge (this already existed in the cpp bridge but was not exposed to fortran yet)
• in the madevent executable, during the first pass on cudacpp bridge that is meant to compute only helicities, I call the bridge with this flag=true: this ensures that only a few events (chosen by cudapp, typically 16) are used for helicity computation
• without this patch, what was happening was that, with 16384 events in the bridge, the first pass was computing MEs for all 16k events, even if only 16 were used to cmpute helicities
• note that the computation scales with SIMD, which explains why it was faster with AVX2 than in no SIMD
• note also that this was adding a huge overhead to the cudacpp calculation with respect to fortran, because fortran does not have the extr acalculation of these 16k events
• and note also that the overhead was reduced when running with only 32 events in the grid, because then this forst pass was only 32 events and not 16k

So I would say that all is understood. This was a nasty performance overhead. Now cudacpp should look a bit better with respect to fortran, especially if only few events are generated

[oliviermattelaer]

I guess that this would make more sense to include this in the master_goodhel
branch since the two are in a way related.

This will allow to move the HELONLY from a boolean to a float and pass limhel to the code to have this selection of helicity to be done as it should.

Or we can merge this one, and then I do the switch in master_goodhel to have that parameter as a float. (but we should not wait for merging master_goodhel anyway)

But can you comment way some file are modified on matrix.f this is weird/bad
(no isue here actually)

Cheers,

[valassi]

I guess that this would make more sense to include this in the master_goodhel
branch since the two are in a way related.

Hi Olivier, thanks for loking at this.

One point: I really think that we should try NOT to have too many master_xxx branches, and rather have everything in master. Otherwise it becomes unmanageable. I work on master, and I also worked on master_june24 (to merge it in master, essentially ready), but I would avoid more branches. (What I try to do is to have several PRs onmaster in parallel, and in case I need one inside another then I do include them, but always trying to make sure that they can be merged to master one after the pther).

This specific PR makes sense against master, I would put it on master. And it does NOT have much to do with the other hel work you are doing in fortran (IIUC), because you are reducing from two fortran helicity calculations to one. Here I am just modifying how many events are used in the cudacpp helicity computation, I am not changing one vs two computations in cudacpp nor am I touching what is done in fortran. So I would really merge it in master.

This will allow to move the HELONLY from a boolean to a float

Uh? This I really do not understand, I should see what you have done. But again, I'd prefer to merge this to master first and then look at the rest. All the tests I am doing for CMS use this now.

Thanks :-)
Andrea

[oliviermattelaer]

One point: I really think that we should try NOT to have too many master_xxx branches, and rather have everything in master.

Approve the merge of the CI, and master_goodhel and ... they will be in master.

The multiplication of master_xxx is just that I have parallel work on therefore multiple PR (which for some weird reason takes a while to be merged). While master_june24 was special the others are quite "normal branch" (and the master_ prefix is just my typical naming of branch indicating from which branch I'm originating from.

Uh? This I really do not understand, I should see what you have done. But again, I'd prefer to merge this to master first and then look at the rest. All the tests I am doing for CMS use this now.

No problem, I will do a master_goodhel_limhel branch then to show you what I want to do for it (but if we merge master_goodhel in master --which should be uncontroversial-- first such that I can do a master_limhel branch).

[valassi]

I have updated this PR including

• the fixes for cms nvcc without nvtx PR Fix cudacpp makefile for installations (e.g. in CMS) where nvcc exists but curand and nvtx headers are missing  #966
• the fix in the mac CI PR CI fix: move from gfortran-11 to gfortran-14 on Mac github-hosted runners #974
• conflict fixes in codegen logs

This should be ready to merge, when the CI tests have passed

[valassi]

This will allow to move the HELONLY from a boolean to a float

Uh? This I really do not understand, I should see what you have done. But again, I'd prefer to merge this to master first and then look at the rest. All the tests I am doing for CMS use this now.

No problem, I will do a master_goodhel_limhel branch then to show you what I want to do for it (but if we merge master_goodhel in master --which should be uncontroversial-- first such that I can do a master_limhel branch).

Hi @oliviermattelaer thanks again for the discussion yesterday. Ok now I understand better what you mean. We could reuse the same parameter as a float in the following way

• if it is >0 or =0: consider this as a helicity filtering call (ie goodhelonly=true), with the float representing LIMHEL, ie start the cudacpp helicity filtering on a subset of events and with that value of LIMHEL (see Allow LIMHEL>0 in cudacpp (using the exact same algorithm as in fortran) #564)
• if it is <0 (eg say -1 by convention as you suggested), then consider this as a normal ME computation, which assumes that helicities have been precomputed.

Note also that I opened #975 about possibly adding channelid to helicity filtering (which now instead is done on all channels).

[valassi]

The CI completed, there are the usual expected 3 failures

Hi @oliviermattelaer as discussed yesterday, I now self-merge this. Some followup work remains to do in

• Allow LIMHEL>0 in cudacpp (using the exact same algorithm as in fortran) #564 (and maybe elsewhere) for passing LIMHEL>0 to cudacpp
• Use channelid in cudacpp (and fortran) helicity filtering? #975 for using channels in helicity filtering

I also still need to review and merge your comprehensive helicity changes in fortran (which I think is #955 and related work)

Thanks Andrea