-
Notifications
You must be signed in to change notification settings - Fork 33
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
Comments
Hi,
This is the kind of stuff that I prefer not to touch. Since the probability to break something is high
and the probability of conflict with other MG5 branch high as well (in particular madnis here).
So a "simple" check of compilation is not good enough (one need to test all the cases where madevent is used
(i.e. all the possible way to build madevent code, like helicity recycling, no grouping, loop-induced, madweight)
I just spend one month to fix all the stuff that we did broke in all those modes so not so motivated to restart such work
(and I spotted another segfault yesterday night :-(, so not yet finished)
Concerning the plugin, my strategy is that we should maintain it in the long term.
and therefore:
1) minimize patch
2) only change what we need to change (so the matrix-element and the bridge)
So I would avoid to do that in the plugin and indeed propose such change via a PR in MG5aMC.
But such PR will need a quite deep and long scrutiny (i.e. I doubt that we can put that in/for 3.6.0 and therefore for the release of the plugin).
Maybe a better PR for the moment is to put a depreciation warning or similar to avoid breaking things and help to build confidence in the fact that this can be removed. We obviously need to be careful given how many person are using this tool.
Cheers,
Olivier
On 15 Aug 2024, at 11:32, Andrea Valassi ***@***.***> wrote:
Hi @oliviermattelaer<https://github.com/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
—
Reply to this email directly, view it on GitHub<#967>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AH6535V2L7ZYQMG2APE6FKDZRRYSPAVCNFSM6AAAAABMR4LS2WVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ3DONZTGIYDGNQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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
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. |
I suggest in any case to at least change this comment, otherwise it is just totally misleading for anyone reading the code
|
…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
…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)
…till we agree on this) Revert "[cmsdyps] in ppdy3j, remove Source/htuple.f madgraph5#967" This reverts commit 1f4ce0b.
…e (from ranmar or htuple) madgraph5#967
…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
…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)
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
The text was updated successfully, but these errors were encountered: