search

madgraph5 / madgraph4gpu

GPU development for the Madgraph5_aMC@NLO event generator software package
29 stars 33 forks source link

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

Closed valassi closed 3 months ago

valassi commented 4 months ago

Hi @oliviermattelaer I have completed many more fixes for SUSY so I am including them here.

This PR includes also all the changes that are in PR #625 (first set of susy changes) and in PR #822 (fix for a helas bug affecting also non SUSY processes). I filed separate PRs to make it easier (maybe) to review.

But if you prefer, I can also just merge this straight away and close the other two.

Can you please review?

Notes

The status is: all SUSY processes that I tried are now correctly code generated, built and tested. Can you maybe also suggest new processes to try?

Thanks Andrea

cc @roiser

valassi commented 4 months ago

Note: in spite of all the SUSY issues this fixes, this still does NOT fix SMEFT builds.

valassi commented 4 months ago

Hi @oliviermattelaer I confirm that this is now ready for review, can you please have a look? Let me know if you want to discuss it via zoom or similar.

As mentioned, this includes also PR #625 and PR #822. So easier to only review this one if you prefer.

oliviermattelaer commented 4 months ago

thanks @valassi sorry to be slow for the moment, I will try to review this one today (or maybe the two others depending of this review I guess).

Sorry,

Olivier

valassi commented 4 months ago

thanks @valassi sorry to be slow for the moment, I will try to review this one today (or maybe the two others depending of this review I guess).

Sorry,

Olivier

Thanks Olivier. Just to clarify (sorry if confusing): this PR is the most advanced of them all. The other two are earlier ones and less complete. But, apart from one issue, the comments you sent to the simpler #625 still make sense here.

One notable thing that was broken in #625 but is fixed here is that previously only real BSM parameters were handled, now instead here also the complex ones are handled.

By the way this confirms what I said in the other comments: somehow, the BSM couplings are NOT handled by the code that handles the cIPC for SM couplings. They go through a diferent part of teh python code, which was broken and which here I fixed.

valassi commented 4 months ago

Hi @oliviermattelaer , as discussed, I have merged #822 and also #625. So now this contains only the additional fixes, which fix SUSY as far as I can tell.

As discussed in #625, before considering this good to merge, I will stil investigate whether I can relax the "if model starts with sm" condition, because this is dangerous and may include things like SM with Z prime, etc.

I also quote your comments from https://github.com/madgraph5/madgraph4gpu/pull/625#issuecomment-1998308440

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--).

I will not address these points yet. I will just try to remove the 'SM' condition

valassi commented 4 months ago

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.

Hi @oliviermattelaer again, this is now done. Thanks for the good suggestion! The code is now cleaner.

En passant, I also identified and fixed a couple more minor issues for EFT. In particular, heft_gg_h now builds and runs ok also in HRDCOD=0 mode (it was not before). The issue here was that I was computing my list of "BSM" independent parameters that must go to device constant memory only looking at those needed to compute dependent couplings, but there are also computations of dependent parameters, and this is now fixed.

Unfortunately the SMEFT processes still have other issues #614 #616 #632, but it looks better anyway.

By the way I added back susy_gg_tt to the repo. There are now both susy_gg_tt (needed to test double BSM parameters, 1 value each) and susy_gg_t1t1 (needed to test complex BSM parameters, 2 values each). And I added these two processes and also heft_gg_h to the CI tests.

This is again ready for review, I remove the draft status. Let me know if you have comments.

I will still run a few tests though before merging. Thanks!

cc @roiser

valassi commented 4 months ago

By the way @oliviermattelaer have you got a better HEFT suggestion that gg>h? This process works but has no phase space as there is no degree of freedom. Maybe something like HEFT gg>bb (with massive bs) would be better? Thanks

valassi commented 4 months ago

Ok I have completed my tests. @oliviermattelaer I think this is ready to be merged when it is reviewed.

Note, I have also implemented the "tmad" tests comparing fortran to cuda/cpp, an dthey fail for both susy_gg_tt #825 and for susy_gg_t1t1 #826. But I suggest meging this one already (which fixes many other things!) and then addressing those other issues later.

By the way, again, eventually I'd like to remove heft_gg_h by something more interesting, since heft seems fixed (unlike smeft).

valassi commented 3 months ago

I will merge to ease the comparison to smeft and then heft.

Keep here the three comments for later:

valassi commented 3 months ago

I will merge to ease the comparison to smeft and then heft.

Keep here the three comments for later:

* what about param.name == 'ZERO' ?

* mdl_g will be return true for mdl_g1 as expression?

* check the status of "FIXME! Here there should be different code generated depending on MGONGPUCPP_NBSMINDEPPARAM_GT_0"?

Ok I have added comments on all three points (previously resolved without answers). All done for me.