Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix "nprocesses>1" code generation (example: uutt within pptt gives "const int denominators = 36,36;") #343

Closed
valassi opened this issue Jan 24, 2022 · 7 comments · Fixed by #396
Assignees

Comments

@valassi
Copy link
Member

valassi commented Jan 24, 2022

Hi @oliviermattelaer as discussed today at the meeting I am assigning this to you.

This is a followup to #337 and to #272. The problem: code generation for pptt now succeeds (after fixing the assert in #337), however the code does not build in the uutt subdirectory, because nprocesses=2 there (#272).

I checked that the uutt standalone and the uutt within pptt differ in the following way for instance:

/data/avalassi/GPU2020/madgraph4gpuX/epochX/cudacpp> diff -r pp_tt.auto/ uu_tt.auto/
...
diff -r pp_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc uu_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc
...
502c497
<     const int denominators = 36,36; // FIXME: assume process.nprocesses == 1 for the moment (eventually denominators[nprocesses]?)
---
>     const int denominators = 36; // FIXME: assume process.nprocesses == 1 for the moment (eventually denominators[nprocesses]?)

From our discussions, I understand that this is because "uutt" within pptt is not only "uu" proper, but also some other combinations of quarks (as you mentioned today, probably it is that u+ubar to t+tbar and ubar+u to t+tbar are not exactly the same?). Anyway, what I mean is that the ggtt within pptt and the ggtt standalone instead are essentially the same code, so it is really the "uutt" part which is different.

As discussed today, this probaby comes from the base python that you developed, rather than from my plugin modifying your python. So best if you take a look. Thanks! Andrea

PS I am instead merging PR #340 that contains ONLY the fix for the assert in #337

valassi added a commit to valassi/madgraph4gpu that referenced this issue Jan 24, 2022
…rocesses

This completes the fix for the assert issue madgraph5#337 (pptt generation was failing).
The second issue in pptt (generation succeeds but build fails with nprocesses=2) is moved to madgraph5#343
@oliviermattelaer
Copy link
Member

So I confirm that the nprocesses=2 is due here to the symmetry between u u~ process and u~ u.
As it is quite clear from the code below:

        calculate_wavefunctions(perm, helicities[ihel]);
        t[0] = matrix_1_uux_ttx();
        // Mirror initial state momenta for mirror process
        perm[0] = 1;
        perm[1] = 0;
        // Calculate wavefunctions
        calculate_wavefunctions(perm, helicities[ihel]);
        // Mirror back
        perm[0] = 0;
        perm[1] = 1;
        // Calculate matrix elements
        t[1] = matrix_1_uux_ttx();

I will prevent such type of duplication of matrix-element for the gpu plugin (I do not see the point to be honest)

@oliviermattelaer
Copy link
Member

I have forbidden the increase of nprocesses due to symmetry factor (will see later if other source of multi-processes occurs)

@valassi
Copy link
Member Author

valassi commented Jan 27, 2022

Thanks Olivier! I created #360 and assigned it to myself about moving to a more recent launchpad version

@valassi valassi changed the title Fix "nprocesses>1" code generation (example: uutt within pptt) Fix "nprocesses>1" code generation (example: uutt within pptt gives "const int denominators = 36,36;") Mar 3, 2022
@valassi
Copy link
Member Author

valassi commented Mar 3, 2022

Just a note, I observed this again while looking at clang-format in #388, but I will ignore it there

@valassi valassi self-assigned this Mar 7, 2022
@valassi valassi reopened this Mar 7, 2022
@valassi
Copy link
Member Author

valassi commented Mar 7, 2022

Reopening. Olivier has fixed this in upstream bazaar, but I need to fix this in the cudacpp plugin.

I have tested that the bazaar "gpu" backend has changed between 270 and 311:

diff -r pp_tt.gpu270 pp_tt.gpu311
...
diff -r pp_tt.gpu270/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc pp_tt.gpu311/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc
3c3
< // MadGraph5_aMC@NLO v. 2.9.5, 2021-08-22
---
> // MadGraph5_aMC@NLO v. 3.3.1_lo_vect, 2022-01-30
297,298c297,298
<     const int nprocesses = 2;  // FIXME: assume process.nprocesses == 1
<     const int denominators[2] = {36, 36}; 
---
>     const int nprocesses = 1;  // FIXME: assume process.nprocesses == 1
>     const int denominators[1] = {36}; 
303a304
> 
345a347
> 

Note in particular that my plugin produces bad code both with 270 and with 311,

const int denominators = 36,36; 

The fix is simple, the denominators should become an array and take the number of subprocesses (which is 1 in the new version, but it still remains an array denominators[1], not a scalar denominators).

valassi added a commit to valassi/madgraph4gpu that referenced this issue Mar 8, 2022
…denominators' by vector 'denominators[1]'
valassi added a commit to valassi/madgraph4gpu that referenced this issue Mar 8, 2022
….1.1_lo_vectorization/madgraph/iolibs/export_cpp.py

(check tkdiff 2.7.0_gpu/madgraph/iolibs/export_cpp.py 3.1.1_lo_vectorization/madgraph/iolibs)
valassi added a commit to valassi/madgraph4gpu that referenced this issue Mar 8, 2022
…LL ASSUME NPROCESSES == 1 ***

Fix codegen templates, regenerate ggtt auto, fix also ggtt manual
@valassi
Copy link
Member Author

valassi commented Mar 8, 2022

This has now being addressed in PR #396.

As mentioned in thePR, however, the code still assumes nprocesses==1. We should find another example with nprocesses>1 (#272) even if "mirror processes" are disabled

@valassi
Copy link
Member Author

valassi commented Mar 8, 2022

This is fixed in #396 that I am about to merge. Closing again.

@valassi valassi closed this as completed Mar 8, 2022
valassi added a commit to valassi/madgraph4gpu that referenced this issue Apr 5, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue Apr 5, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue Apr 5, 2023
…1_gux_ttxux to P1_gu_ttxu

The gqttq tests fail anyway and will need to be fixed (madgraph5#630).
However, this completes the addition of gq_ttq as a new process to the repo.
In particular it includes proof that Olivier's "split_nonidentical_grouping" madgraph5#619 fixes the gqttq builds.
It also includes a lot of cleanup for "nprocesses" (madgraph5#272 and madgraph5#343)

Revert "[gqttq] retry the tmad gqttq test with the P1_gu_ttxu directory - the test continues to fail (madgraph5#630)"
This reverts commit 2dea1f7.

Revert "[gqttq] temporarely use P1_gu_ttxu instead of P1_gux_ttxux for gqttq tmad tests"
This reverts commit ea23a9a.
valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this issue Aug 16, 2023
…esses as in 3.1.1_lo_vectorization/madgraph/iolibs/export_cpp.py

(check tkdiff 2.7.0_gpu/madgraph/iolibs/export_cpp.py 3.1.1_lo_vectorization/madgraph/iolibs)
valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this issue Aug 16, 2023
…B HOWEVER STILL ASSUME NPROCESSES == 1 ***

Fix codegen templates, regenerate ggtt auto, fix also ggtt manual
valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this issue Aug 16, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue Oct 28, 2023
…matting, and especially the build will fail.

Codebase includes merging commit a6731bd (Olivier Wed Aug 23 13:23:12 2023 +0200)
This uses Olivier's 'fix_mirror' branch for PR madgraph5#754

In particular
  a6731bd       Olivier Mattelaer       Wed Aug 23 13:23:12 2023 +0200  Merge branch 'fix_mirror'
  2556cdd       Olivier Mattelaer       Wed Aug 23 09:27:38 2023 +0200  avoid that mirroring is reset by the plugin

These lines fail the build (as well as clang formatting)
[NOT OK] Check formatting in: pp_tt012j.mad/SubProcesses/P0_uux_ttx/CPPProcess.cc
786c786
<     constexpr int helcolDenominators[1] = { 36,36 }; // assume nprocesses == 1 (madgraph5#272 and madgraph5#343)
---
>     constexpr int helcolDenominators[1] = { 36, 36 }; // assume nprocesses == 1 (madgraph5#272 and madgraph5#343)
The same happens in each P subdirectory.

Build errors:
  ccache /usr/local/cuda-12.0/bin/nvcc   -Xcompiler -O3 -lineinfo -I. -I../../src -I/usr/local/cuda-12.0/include/ -DUSE_NVTX -gencode arch=compute_70,code=compute_70 -gencode arch=compute_70,code=sm_70 -use_fast_math -std=c++17  -ccbin /usr/lib64/ccache/g++ -DMGONGPU_FPTYPE_DOUBLE -DMGONGPU_FPTYPE2_DOUBLE -Xcompiler -fPIC -c gCPPProcess.cu -o gCPPProcess.o
  gCPPProcess.cu(779): error: static assertion failed with "Assume nprocesses == 1"
  gCPPProcess.cu(786): error: too many initializer values
  2 errors detected in the compilation of "gCPPProcess.cu".
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants