-
Notifications
You must be signed in to change notification settings - Fork 32
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
Further fixes/improvements for iconfig-channel mappings in coloramps.h #877
Conversation
…p and gpucpp_826, to allow cherry-picking Olivier's fix_826 changes (later on, will include Olivier's gpucpp_826 change into gpucpp directly) Revert "[tmad] update mg5amcnlo to f274cab55, adding volatile to prevent madgraph5#855 crashes in rotxxx (move this upstream as suggested by Olivier)" This reverts commit 720ae02. Revert "[valgrind] upgrade MG5AMC to include the merge of PR madgraph5#110 and PR madgraph5#112 into the gpucpp branch" This reverts commit 7d3dc34. Revert "[valgrind] upgrade MG5AMC to include the workaround for uninitialised values mg5amcnlo/mg5amcnlo#111" This reverts commit f355965. Revert "[valgrind] upgrade MG5AMC to include the fix for memory leak mg5amcnlo/mg5amcnlo#109" This reverts commit 7bb4142.
… now including Olivier's extra commit previously in gpucpp_826 This should complete the merge in madgraph4gpu master of the gpucpp_826 and gpucpp branches of MG5AMC. It should now be possible to remove the gpucpp_826 branch from MG5AMC.
… in Olivier's patch PR madgraph5#852 for issue madgraph5#826 in susy_gg_t1t1
…y branch (WIP PR madgraph5#853) to allow cherry-picking the next commit git checkout 335d6d2 susy_gg_t1t1.mad/CODEGEN_mad_susy_gg_t1t1_log.txt
…rt (just check that the merge so far was ok)
Revert "[color] temporarely regenerate susy_gg_t1t1.mad, no change, will revert (just check that the merge so far was ok)" This reverts commit dbfb3bc.
…rangely, clang formatting was not complaining?)
…clang formatting was not complaining?)
…files checked with clang formatting
… comma is allowed in the std::map initializer list
… allowed in the std::map initializer >
Note: cuda compilation 'make -j BACKEND=cuda' fails with coloramps.h(14): error: dynamic initialization is not supported for a __device__ variable
…p as constexpr, this fails with a different error ccache /usr/local/cuda-12.0/bin/nvcc -I. -I../../src -Xcompiler -O3 -gencode arch=compute_70,code=compute_70 -gencode arch=compute_70,code=sm_70 -lineinfo -use_fast_math -I/usr/local/cuda-12.0/include/ -DUSE_NVTX -std=c++17 -ccbin /usr/lib64/ccache/g++ -DMGONGPU_FPTYPE_DOUBLE -DMGONGPU_FPTYPE2_DOUBLE -Xcompiler -fPIC -c -x cu CPPProcess.cc -o CPPProcess_cuda.o coloramps.h(14): error: expression must have a constant value coloramps.h(14): note #2703-D: cannot call non-constexpr function "std::map<_Key, _Tp, _Compare, _Alloc>::map(std::initializer_list<std::map<_Key, _Tp, _Compare, _Alloc>::value_type>, const _Compare &, const std::map<_Key, _Tp, _Compare, _Alloc>::allocator_type &) [with _Key=int, _Tp=int, _Compare=std::less<int>, _Alloc=std::allocator<std::pair<const int, int>>]" /usr/include/c++/11/bits/stl_map.h(228): here coloramps.h(14): error: a constexpr variable must have a literal type or a reference type CPPProcess.cc(1012): error: no operator "[]" matches these operands operand types are: const std::map<int, int, std::less<int>, std::allocator<std::pair<const int, int>>> [ const unsigned int ] 3 errors detected in the compilation of "CPPProcess.cc".
…making the map constexpr Revert "[susy] in susy_gg_t1t1.mad, try to fix coloramps.h by defining the map as constexpr, this fails with a different error" This reverts commit 38b3afd.
…p into a constexpr int array
…constexpr int array
…ncludes clang format fixes) This code now compiles... but it does not seem to fix the empty cross section problem
…graph5#826 is NOT fixed) after Olivier's first two commits in PR madgraph5#852 ./tmad/teeMadX.sh -susyggt1t1 +10x -makeclean (PS: it is NOW quite obvious to me that PR madgraph5#852 should be tested against different issues like ggttgg madgraph5#856, NOT against susy madgraph5#826) (PS: will now therefore slowly move to different tests - first, however, recover my coloramps.h proposal in susy and Olivier's later changes)
…is ok - will revert
…d by Olivier in PR madgraph5#877 (undo fix for madgraph5#856 in mg5amcnlo/mg5amcnlo#116)
…moving the icolamp fix from MG5aMC to cudacpp NB: this is now functionally equivalent to the previous final version a245daf of color2 Will now remove the fix for madgraph5#856 as suggested by Olivier for PR madgraph5#877 - tests will fail
…5#856 as suggested by Olivier in PR madgraph5#877 Will not rerun all tput/tmad tests manually as many are expected to fail. The CI will show many failing tests (variants of madgraph5#856).
I have now done this, so that we can finally merge this and PR #852. For the moment I moved back to mg5amcnlo/mg5amcnlo@09c96dd. I will later move to the latest gpucpp commit mg5amcnlo/mg5amcnlo@1e2aa4b as suggested by Olivier. I have removed the association to #856 because THIS PR WILL NOT FIX #856. It contains a fix (moved to cudacpp), but then also its reverting. The CI test shows 9 errors in https://github.com/madgraph5/madgraph4gpu/actions/runs/9776063937
(I also suspect more errors with LHE files similar to 856 would show up if running on different iconfig). As suggested by Olivier I will include my suggested fix for 856 in my own master_valassi fork, which I will then need to create. I will add back the bypass for 856, so that the CI checks for new issues only. |
…ed by Olivier in PR madgraph5#877 (undo fix for madgraph5#856 in mg5amcnlo/mg5amcnlo#116)
…xcept for cosmtic formatting) from export_cpp (Prepare to move into cudacpp the fix for madgraph5#856 previously in unmerged PR mg5amcnlo/mg5amcnlo#116)
…and for mg5amcnlo/mg5amcnlo#114 which was previously in mg5amcnlo/mg5amcnlo#116
…moving the icolamp fix from MG5aMC to cudacpp NB: this is now functionally equivalent to the previous final version a245daf of color2 Will now remove the fix for madgraph5#856 as suggested by Olivier for PR madgraph5#877 - tests will fail
…#114 as suggested by Olivier - TMAD TESTS WILL FAIL Revert "[color2] in CODEGEN model_handling.py, add the fix for madgraph5#856 and for mg5amcnlo/mg5amcnlo#114 which was previously in mg5amcnlo/mg5amcnlo#116" This reverts commit 2daad8f. Revert "[color2] in CODEGEN model_handling.py, add get_icolamp_lines as-is (except for cosmtic formatting) from export_cpp" This reverts commit 705b5eb.
…5#856 as suggested by Olivier in PR madgraph5#877 Will not rerun all tput/tmad tests manually as many are expected to fail. The CI will show many failing tests (variants of madgraph5#856).
…es for known issues The CI tests are expected to fail for madgraph5#826 in susy and madgraph5#872 in pp_tt012j In addition, madgraph5#856 and similar issues with LHE color mismatches are expected to appear
…aph5#856 (commented out) to known issues in the CI as its fix has been removed
…bc3 in the gpucpp branch (improving the comments around 'goodjet' valgrind warnings mg5amcnlo/mg5amcnlo#111)
… minor cosmetic changes in reweight.f files
…s/testsuite_oneprocess.sh, reenaable bypasses for known issues - the CI is now expected to pass
I have now moved to the latest gpucpp commit as suggested by Olivier.
I have merged PR #852 "fix_826". Since Olivier approved this (an earlier version - I believe this has the further improvements he required) I will now merge this one as well, with further fixes/improvements.
Specifically, this is now done. Thanks a lot again Olivier for all the patient discussion on this |
…adgraph5#852 and color2 PR madgraph5#877, instead of color PR madgraph5#873) into fpe
… that madgraph5#856 is not yet fixed in upstream/master, as expected after madgraph5#877 ./tmad/teeMadX.sh -ggttgg -makeclean +10x
…er merging Note: these are the tmad logs where madgraph5#856 was fixed in a previous attempt during madgraph5#877 git checkout 46356d6 tmad/logs_*
… that madgraph5#856 is not yet fixed in upstream/master, as expected after madgraph5#877 ./tmad/teeMadX.sh -ggttgg -makeclean +10x
…er merging Note: these are the tmad logs where madgraph5#856 was fixed in a previous attempt during madgraph5#877 git checkout 46356d6 tmad/logs_*
Hi @oliviermattelaer this is a modified version of #873, to be compared to your fix_826. It is built on top of your fix_826 PR #852, so it has by construction no conflicts with that. (So it can be compared to fix_826 as you suggested in #873 (comment))
As discussed offline I would suggest to agree upfront on this one and on mg5amcnlo/mg5amcnlo#116, and then:
Thanks!
Andrea