fix susy gg_tt builds and add it to the repo

[valassi]

This is a followup to #624, where I fixed the code generation of SUSY gg_tt. Here I am also trying to fix the builds, but there are issues, some of which will remain "unfixable" for a while (I will post details: trigonometric functions are not constexpr; and FFV function calls should not include "-COUPS" as these are pointers not values).

Note also, the main reason why I started going into susy processes with PR #624 and this new PR here is that I wanted to check "nprocesses>1" in #272, before PR #619. This PR is currently also including some SM processes to that effect (I will probably split them away).

[valassi]

I have cleaned this up, splitting other stuff like the SM gq_ttq to PR #626

[valassi]

For the record also keep in mind this comment by Olivier
#272 (comment)
"also it seems that some matrix element are missing (the symmetric ones) in the susy case, this need to be fix."

[valassi]

I have just merged the latest master into this PR (which contains a new sa generated code dir, so there are no conflicts).

Note that the code was generated with old code, so this cannot be directly trasnposed to codegen

[valassi]

I have updated this PR to our latest code bases and updated the PR. A few findings

• Code generation for susy gg->tt succeeds

• In HRDCOD=0 mode, the build fails (but I need to investigate... in any case there is a hardcoded "use HRDCOD=1 instead" to remove)

• In HRDCOD=1 moe, nvcc builds fail because sin/cos/atan are not constexpr (see SUSY HRDCOD=1 builds fail because sin/cos/atan are not constexpr (only CUDA fails, while C++ succeeds) #627 )

• However in HRDCOD=1 mode, no-CUDA builds with C++ only do succeed! See 415e27b and d5ded87. This might mean that it might be possible to hardcode the constexpr implementation from gcc into the nvcc part of our code if we wanted this?

In any case, the real goal here (also to allow @zeniheisser 's reweighting) is to allow HRDCOD=0 builds.

PS A few changes were necessary. These are NOT YET in codegen. I only modified the generated susy_gg_tt here. This remains well in WIP anyway.

[valassi]

A further update for HRDCOD=0.

• I have "temporarely" removed the error "you should use HRDCOD=1", see 717a5de: this needs a cleanup, make it a warning? only for cuda?
• I have fixed the first of two issues, the visibility of mdl_I51x11, see df8b87a. I had to use a getInstance() accessor to a static class. This goes through in c++, but it gives a warning in cuda, which I suspect is the reason why MEs are all 0 in cuda at runtime,
• I have fixed the second of two issues, by defining an "indices" variable that was commented out, see 4f3b83a. This is ok here, but maybe should remain commented out in other codes where this is not needed (or an unused variable warning must be silenced?). This neeeds some cleanup before the backport to codegen.

Anyway, all in all, this is now the matrix of where we are with this:

• C++ builds and tests are now fully ok both with HRDCOD=0 and HRDCOD=1. This is very good, so I guess this PR can evolve from WIP and be merged at some point.
• CUDA builds fail with HRDCOD=1 because of sin/cos/atan not constexpr (SUSY HRDCOD=1 builds fail because sin/cos/atan are not constexpr (only CUDA fails, while C++ succeeds) #627 )
• CUDA builds succeed with HRDCOD=0, but MEs are all 0 at runtime. I guess that a better structure of parameter classes is needed here, to avoid that getInstance which host/device code does not like? Or this mdl_I51x11 must be also copied from the host to the device? I should cross check again where the alphas running happens in cuda, and if it is on the host then it is just the coupling that must be transferred to the device...

[valassi]

Hi @oliviermattelaer I confirm that this is ready to be merged.

NB This fixes gg_tt and gq_ttq in SUSY (so not even SUSY processes itself), and partly gg_gogo. But many other issues remain, see #820. I would still suggest to merge this one already, and then investigate the rest later on.

[valassi]

Note: this is a standalone PR but is also included in #824, which adds additional fixes for SUSY.

Rephrasing:

• after this PR fix susy gg_tt builds and add it to the repo #625 some SUSY issues are fixed, but there are many pending
• after PR comprehensive set of fixes for SUSY (complete) and EFT (partial) - keep susy_gg_tt, susy_gg_t1t1, heft_gg_h in repo and test them in CI #824, all SUSY issues I identified are fixed

[oliviermattelaer]

Here I'm struggling to understand two points (sorry for that)

• what is problematic/special with the width of majorana particle? (i.e. do we really need to separate those from the other widths?)
• What is the issue with BSM coupling? They should also split in running/non running coupling (like the SM does)?

Typically, I try to avoid to single out SM compare to BSM model (my typical strategy is saying that SM is a BSM model like any other), but this is not a require strategy.
However, using the model name to check for SM model is likely a bad idea since model can be loop_sm and still be SM ... (and the opposite is True too, as written in one comment)

I will check some of the issue linked to see if I can figure it out on my own but comment on this already (maybe I should have done the opposite, sorry in case)

[valassi]

Here I'm struggling to understand two points (sorry for that)

No need to be sorry!

* what is problematic/special with the width of majorana particle? (i.e. do we really need to separate those from the other widths?)

What is special is that there is a need for some code that does "if mass<0 then width=-width", i.e. if the mass is less than 0 then also the width is less than 0.

What is problematic is that, before various fixes, there were some code generation errors. I saw this in #622 for cudacpp, while Walter and Nathan also sa it in #694 for the SYCL plugin.

Now, why I chose to split one python method into two methods "without Majorana fixes" and "only apply Majorana fixes", unfortunately I do not remember. Maybe I could have kept those in the same method, I am not sure. The good thing is that the patch works and fixed some issues.

On top of this, in addition to #622 codegen error "ValueError : not enough values to unpack", later on I got some other errors specific to this code. One was for instance that, in the HRDCOD=1 setup, you cannot right "if m<0 then w=-w", because this is not a constexpr assignment. So I rewrote this as "w = ( m<0 ? -absw : + absw )" or something similar.

Finally, note that the "if mass<0 then width=-width" machinery is something that I found in the original code, I did not write it myself (it comes from UFO? from code you wrote? no idea...). I just made sure that the logic works ok now in all codegen and builds.

Does this answer your question?

* What is the issue with BSM coupling? They should also split in running/non running coupling (like the SM does)?

Listen, here I also have a hard time myself. I have done enough reverse engineering to fix the issues, but I must say that I still do not understand the logic. "Experimentally", I just see that BSM models introduce some parameters that are handled by very different parts of the python code, at different moments in time.

We should probably sit down and take a look at some point...?

If you want to get a better idea, anyway, I suggest you look at what appears in a generated SUSY process. For instance compare susy gg to stop pairs with SM gg to top pairs? This is from the more recent PR #824,
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/susy_gg_t1t1.sa/src/Parameters_MSSM_SLHA2.h
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/susy_gg_t1t1.sa/src/Parameters_MSSM_SLHA2.cc
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/susy_gg_t1t1.sa/SubProcesses/P1_Sigma_MSSM_SLHA2_gg_t1t1x/CPPProcess.cc
vs
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/gg_tt.sa/src/Parameters_sm.h
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/gg_tt.sa/src/Parameters_sm.cc
https://github.com/valassi/madgraph4gpu/blob/susy/epochX/cudacpp/gg_tt.sa/SubProcesses/P1_Sigma_sm_gg_ttx/CPPProcess.cc

What is especially interesting are the Parameters.h and the CPPProcess.cc files. (The Parameters.cc is less interesting).

I think that in the end there are three main differences? maybe I am missing some

• the majorana stuff already discussed, something here is unavoidable
• the fact that SIMD vectors of floats need some special handling (issue Fix heft generation after alphas PR: more fundamentally, remove reading of param_card.dat? #439... this I fixed almost two years ago, May 2022, so I do not remember the details), something here is also unavoidable I guess
• the fact that there are some parameters double mdl_bsmIndepParam[nBsmIndepParam] = { mdl_I52x33.real(), mdl_I52x33.imag(), mdl_I51x33.real(), mdl_I51x33.imag(), mdl_I75x33.real(), mdl_I75x33.imag(), mdl_I74x33.real(), mdl_I74x33.imag(), mdl_I52x36.real(), mdl_I52x36.imag(), mdl_I51x36.real(), mdl_I51x36.imag() }; that I copy to constant memory like we do for cIPC SM parameters, but which do not go through the same code... a legitimate question is, "could we not handle those like the cIPC?", the answer maybe is yes, but somehow the code that manages the cIPC does not manage them...? again, I got lost in the overall logic of the python, which function controls which parameters

Along the same lines, one other thing that so far I was unable to understand is the order in which these parameters are discovered, and the collaboration bwteen the UFOconverter, the HelasWriter and the other classes. In particular, I have this ugly comment, which shows that I do not completely master the order of generation of Parameters.h and CPPProcess.cc

  // AV Jan 2024 (PR #625): this ugly #define was the only way I found to avoid creating arrays[nBsm] in CPPProcess.cc if nBsm is 0
  // The problem is that nBsm is determined when generating Parameters.h, which happens after CPPProcess.cc has already been generated
  // For simplicity, keep this code hardcoded also for SM processes (a nullptr is needed as in the case nBsm == 0)
#ifdef MGONGPUCPP_NBSMINDEPPARAM_GT_0

Anyway, as above: it works now! So, it can be kept as a cleanup for later... unless you see obvious ways to improve this fast.

Typically, I try to avoid to single out SM compare to BSM model (my typical strategy is saying that SM is a BSM model like any other), but this is not a require strategy. However, using the model name to check for SM model is likely a bad idea since model can be loop_sm and still be SM ... (and the opposite is True too, as written in one comment)

Ok yes I agree, as written in the comment above. I can try to apply the "special handling" all the time, and see if it still produces SM code which makes sense.

I will check some of the issue linked to see if I can figure it out on my own but comment on this already (maybe I should have done the opposite, sorry in case)

Voila I commented on everything, hope this helps. I am sure you have many comments on my comments...

Thanks!

[oliviermattelaer]

Ok thanks a lot,

This is helpfull, Maybe the only change that need to be done (but not required to be done in this PR) is to run all the BSM trick for the SM model as well and check that SM is still working correctly.

We can also sit together on Friday to see if I can design a quick and nice way to fix your issue with those coupling. But, I guess that the answer will not be quick...

One reason of the complexity is that the structure of the code is likely:

1. read the UFO model -> convert it to a MG5 object
2. potential optimization of the model by MG5
3. generate the diagrams
4. generate the helas call -> write the process file
5. retrieve from step 3 or 4 the list of couplings/lorentz structure needed for the particular process and create code to compute only such list. (Note that all parameters are handle separetly and likely do not have such optimization ...)

So I guess the confusion might partly comes from the difference between coupling and parameter (and the difference is in a way arbitrary) and the fact that some parameter can/should be real (coupling should always be treated as complex --even if they might sometimes/often be real or pure imaginary--).

But in itself you can consider this as an approve :-)

Cheers and thanks,

Olivier

[valassi]

Ok thanks a lot,

This is helpfull, Maybe the only change that need to be done (but not required to be done in this PR) is to run all the BSM trick for the SM model as well and check that SM is still working correctly.
...

But in itself you can consider this as an approve :-)

Cheers and thanks,

Olivier

Hi Olivier, thanks!
LIsten, I would do the following:

• consider this PR fix susy gg_tt builds and add it to the repo #625 as approved and merge it
• then work more on PR comprehensive set of fixes for SUSY (complete) and EFT (partial) - keep susy_gg_tt, susy_gg_t1t1, heft_gg_h in repo and test them in CI #824 and try to include the BSM tricks also in all SM and see what it gives
• after I merge fix susy gg_tt builds and add it to the repo #625, it becomes easier for you to review the additional changes that are already in PR comprehensive set of fixes for SUSY (complete) and EFT (partial) - keep susy_gg_tt, susy_gg_t1t1, heft_gg_h in repo and test them in CI #824 (and any new changes I will add)

Does this sound ok?

We can also sit together on Friday to see if I can design a quick and nice way to fix your issue with those coupling. But, I guess that the answer will not be quick...

Thanks, it would help to sit down together. But today I am just too busy, maybe next week?

One reason of the complexity is that the structure of the code is likely:

1. read the UFO model -> convert it to a MG5 object

2. potential optimization of the model by MG5

3. generate the diagrams

4. generate the helas call -> write the process file

5. retrieve from step 3 or 4 the list of couplings/lorentz structure needed for the particular process and create code to compute only such list. (Note that all parameters are handle separetly and likely do not have such optimization ...)

So I guess the confusion might partly comes from the difference between coupling and parameter (and the difference is in a way arbitrary) and the fact that some parameter can/should be real (coupling should always be treated as complex --even if they might sometimes/often be real or pure imaginary--).

I will add these comments to #824 so that we have them there for further discussion.

[valassi]

Ok thanks a lot,
This is helpfull, Maybe the only change that need to be done (but not required to be done in this PR) is to run all the BSM trick for the SM model as well and check that SM is still working correctly.
...
But in itself you can consider this as an approve :-)
Cheers and thanks,
Olivier

Hi Olivier, thanks! Listen, I would do the following:

* consider this PR #625  as approved and merge it

I will do the above: consider this as approved, and merge it.

I will move the pending comments and todo to #824.