Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove htuple.f from generated code #967

Open
valassi opened this issue Aug 15, 2024 · 4 comments · May be fixed by #970 or #946
Open

Remove htuple.f from generated code #967

valassi opened this issue Aug 15, 2024 · 4 comments · May be fixed by #970 or #946
Assignees

Comments

@valassi
Copy link
Member

valassi commented Aug 15, 2024

Hi @oliviermattelaer I just realised that there are two functions ntuple with the same API, one in ranmar.f and one in htuple.f.

This is extremely confusing. In fact yesterday evening I thought that all of madgraph was using the htuple algorithm (a quasi random generator based on scrambled hanson) rather than ranmar. This was also rather scary because there is no seed change in htuple.f.

Now I think I understand that the ranmar version is used (and randinit is propagated correctly there).

So my suggestion is, can we remove htuple.f from generated code? I can do that, easy, but I'd need a confirmation please.

I did a quick test where I removed htuple.f and in fact the code builds fine.

Next question would be, should htuple.f be removed in upstream mg5amcnlo or in the plugin... I can do both

Thanks
Andrea

@oliviermattelaer
Copy link
Member

oliviermattelaer commented Aug 15, 2024 via email

valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 15, 2024
CUDACPP_RUNTIME_DISABLEFPE=1 ./build.cuda_d_inl0_hrd0/madevent_cuda < /tmp/avalassi/input_dy3j_x1_cudacpp
 Found          997  events.
 Wrote           59  events.
 Actual xsec    5.9274488566377981
 [COUNTERS] PROGRAM TOTAL                         :    5.2411s
 [COUNTERS] Fortran Other                  (  0 ) :    0.1763s
 [COUNTERS] Fortran Initialise(I/O)        (  1 ) :    0.0699s
 [COUNTERS] Fortran Random2Momenta         (  3 ) :    3.9639s for  1170103 events => throughput is 3.39E-06 events/s
 [COUNTERS] Fortran PDFs                   (  4 ) :    0.1034s for    49152 events => throughput is 2.10E-06 events/s
 [COUNTERS] Fortran UpdateScaleCouplings   (  5 ) :    0.1345s for    16384 events => throughput is 8.21E-06 events/s
 [COUNTERS] Fortran Reweight               (  6 ) :    0.0526s for    16384 events => throughput is 3.21E-06 events/s
 [COUNTERS] Fortran Unweight(LHE-I/O)      (  7 ) :    0.0643s for    16384 events => throughput is 3.92E-06 events/s
 [COUNTERS] Fortran SamplePutPoint         (  8 ) :    0.1426s for  1170103 events => throughput is 1.22E-07 events/s
 [COUNTERS] PROGRAM SampleGetX             ( 10 ) :    2.4594s for 15211307 events => throughput is 1.62E-07 events/s
 [COUNTERS] CudaCpp Initialise             ( 11 ) :    0.4724s
 [COUNTERS] CudaCpp Finalise               ( 12 ) :    0.0268s
 [COUNTERS] CudaCpp MEs                    ( 19 ) :    0.0342s for    16384 events => throughput is 2.09E-06 events/s
 [COUNTERS] OVERALL NON-MEs                ( 21 ) :    5.2069s
 [COUNTERS] OVERALL MEs                    ( 22 ) :    0.0342s for    16384 events => throughput is 2.09E-06 events/s
@valassi valassi linked a pull request Aug 15, 2024 that will close this issue
@valassi valassi linked a pull request Aug 15, 2024 that will close this issue
@valassi
Copy link
Member Author

valassi commented Aug 15, 2024

Hi Olivier, thanks.

Let's keep this open then. I think that removing this in the plugin is safe (especially if at some point we play with curand or even just vectorizing phase space the htuple.f is completely useless). But apart from developer confusion there is not much harm here... so ok if you prefer not to.

@valassi
Copy link
Member Author

valassi commented Aug 17, 2024

I suggest in any case to at least change this comment, otherwise it is just totally misleading for anyone reading the code

epochX/cudacpp/pp_dy3j.mad/Source/dsample.f /tmp/git-blob-CrupNA/dsample.f ffbab045d5b5f611dd06879e382399998832abd2 100644 epochX/cudacpp/pp_dy3j.mad/Source/dsample.f 0000000000000000000000000000000000000000 100644
740c740
<       data ituple/1/             !1=htuple, 2=sobel 
---
>       data ituple/1/             !1=ntuple(ranmar or htuple), 2=sobel 

valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 19, 2024
…nts madgraph5#967 and additional counters

The only files that still need to be patched are
- 4 in patch.common: Source/makefile, Source/genps.inc, Source/dsample.f, SubProcesses/makefile
- 4 in patch.P1: auto_dsig1.f, auto_dsig.f, driver.f, matrix1.f

./CODEGEN/generateAndCompare.sh gg_tt --mad --nopatch
git diff --no-ext-diff -R gg_tt.mad/Source/makefile gg_tt.mad/Source/genps.inc gg_tt.mad/SubProcesses/makefile gg_tt.mad/Source/dsample.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common
git diff --no-ext-diff -R gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig1.f gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig.f gg_tt.mad/SubProcesses/P1_gg_ttx/driver.f gg_tt.mad/SubProcesses/P1_gg_ttx/matrix1.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.P1
git checkout gg_tt.mad
valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 19, 2024
…nts madgraph5#967 and additional counters

The only files that still need to be patched are
- 4 in patch.common: Source/makefile, Source/genps.inc, Source/dsample.f, SubProcesses/makefile
- 4 in patch.P1: auto_dsig1.f, auto_dsig.f, driver.f, matrix1.f

./CODEGEN/generateAndCompare.sh gg_tt --mad --nopatch
git diff --no-ext-diff -R gg_tt.mad/Source/makefile gg_tt.mad/Source/genps.inc gg_tt.mad/SubProcesses/makefile gg_tt.mad/Source/dsample.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common
git diff --no-ext-diff -R gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig1.f gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig.f gg_tt.mad/SubProcesses/P1_gg_ttx/driver.f gg_tt.mad/SubProcesses/P1_gg_ttx/matrix1.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.P1
git checkout gg_tt.mad

(Fix conflicts in cherry-pick: epochX/cudacpp/CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common)
@valassi valassi linked a pull request Aug 19, 2024 that will close this issue
@oliviermattelaer
Copy link
Member

Done:
mg5amcnlo/mg5amcnlo@a94da76

valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 20, 2024
…till we agree on this)

Revert "[cmsdyps] in ppdy3j, remove Source/htuple.f madgraph5#967"
This reverts commit 1f4ce0b.
valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 20, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 20, 2024
…nts madgraph5#967 and additional counters

The only files that still need to be patched are
- 4 in patch.common: Source/makefile, Source/genps.inc, Source/dsample.f, SubProcesses/makefile
- 4 in patch.P1: auto_dsig1.f, auto_dsig.f, driver.f, matrix1.f

./CODEGEN/generateAndCompare.sh gg_tt --mad --nopatch
git diff --no-ext-diff -R gg_tt.mad/Source/makefile gg_tt.mad/Source/genps.inc gg_tt.mad/SubProcesses/makefile gg_tt.mad/Source/dsample.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common
git diff --no-ext-diff -R gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig1.f gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig.f gg_tt.mad/SubProcesses/P1_gg_ttx/driver.f gg_tt.mad/SubProcesses/P1_gg_ttx/matrix1.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.P1
git checkout gg_tt.mad
valassi added a commit to valassi/madgraph4gpu that referenced this issue Aug 22, 2024
…nts madgraph5#967 and additional counters

The only files that still need to be patched are
- 4 in patch.common: Source/makefile, Source/genps.inc, Source/dsample.f, SubProcesses/makefile
- 4 in patch.P1: auto_dsig1.f, auto_dsig.f, driver.f, matrix1.f

./CODEGEN/generateAndCompare.sh gg_tt --mad --nopatch
git diff --no-ext-diff -R gg_tt.mad/Source/makefile gg_tt.mad/Source/genps.inc gg_tt.mad/SubProcesses/makefile gg_tt.mad/Source/dsample.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common
git diff --no-ext-diff -R gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig1.f gg_tt.mad/SubProcesses/P1_gg_ttx/auto_dsig.f gg_tt.mad/SubProcesses/P1_gg_ttx/driver.f gg_tt.mad/SubProcesses/P1_gg_ttx/matrix1.f > CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.P1
git checkout gg_tt.mad

(Fix conflicts in cherry-pick: epochX/cudacpp/CODEGEN/PLUGIN/CUDACPP_SA_OUTPUT/MG5aMC_patches/PROD/patch.common)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants