Include mg5amcnlo in madgraph4gpu as a git submodule #752

valassi · 2023-08-16T05:43:26Z

Hi @oliviermattelaer @hageboeck @roiser @zeniheisser @Jooorgen
I have completed the inclusion of mg5amcnlo in madgraph4gpu as a git submodule.

This MR is ready to be merged... but please let me know if you have comments or would have liked some things differently.

This completes our "step 1" agreed last week

Step 1, mg5amcnlo is a submodule of madgraph4gpu, while cudacpp is still a directory in the madgraph4gpu repo
Step 2, cudacpp is a separate repo, included as a submodule in mg5amcnlo, which is itself a submodule of madgraph4gpu

For info, what I personally did was the following

by default, mg5amcnlo will be in detached HEAD state with a given commit (no branch)
I went and added a remote for my fork, and then checked out the valassi/gpucpp branch from my fork as a local "valassi_gpucpp" branch which contains the commit. Then updating the module must be done with the merge option, git submodule update --remote --merge.
See section "Working on a Submodule" in https://git-scm.com/book/en/v2/Git-Tools-Submodules

I am also progressing on step2, i.e. the move of the codegen plugin to a separate repo (the only tricky bit is I am keeping the commit history for that and will try to change the links to existing github issues to point to madgraph4gpu - and then making a few tests to understand how to continue porting codegen changes in existing MRs like HIP). It should be ready in one day or two I think.

Thanks
Andrea

PS I take it back slightly (see https://github.com/madgraph5/madgraph4gpu/pull/752/files)

the commit is certainly fixed in the submodule at "Submodule mg5amcnlo added at 941832"
however the .gitmodule file also contains gpucpp
So I guess that a new clone will checkout gpucpp and use the specific commit there? I will check

mkdir <top>/MG5aMC cd <top>/MG5aMC git submodule add https://github.com/mg5amcnlo/mg5amcnlo

cd <top> git submodule set-branch --branch gpucpp MG5AMC/mg5amcnlo (NB: the module name is MG5AMC/mg5amcnlo, you should not run this command with argument mg5amcnlo from directory MG5AMC) (NB: 'git config -l' shows no difference)

git submodule update --remote Submodule path 'MG5aMC/mg5amcnlo': checked out '941832b9b03213da8ab994427add396673820799'

… the mg5amcnlo submodule in this repo

…ories to $MG5AMC_HOME/../TMPOUT

…5amcnlo/PLUGINS after code generation

…er files showing directory names In proc_card_mg5.dat -output madevent CODEGEN_mad_gg_tt ... +output madevent ../TMPOUT/CODEGEN_mad_gg_tt ... In me5_configuration.txt -#mg5_path = /data/avalassi/GPU2023/MG5aMC/ghav-mg5amcnlo +#mg5_path = /data/avalassi/GPU2023/madgraph4gpuMOD/MG5aMC/mg5amcnlo In py3_model.pkl (binary files differ) NOTE: I also added gg_tt.mad/bin/internal/plugin_run_card which seems to be a new file? This is probably something added by OLivier after last week's discussions, it contains {'name':'cudacpp_backend', 'value':'CPP', 'include':False, 'hidden':False}

… by git submodules Note: they were pointing to branch.GIT = gpucpp commit.GIT = 941832b9b Note: 'git show b145ba4' gives Submodule MG5aMC/mg5amcnlo 1a13d8f53..941832b9b: That is to say, the submodule points to the same commit (by default in detached HEAD state, unless users checkout a specific branch)

…ch.GIT

…d for all of them)

…PP_OUTPUT when copying to mg5amcnlo

… all ok, no change except log

…- all ok, no change except log

…UTPUT - all ok, no change except log

…ing plugin as CUDACPP_OUTPUT - all ok, no change except log

valassi · 2023-08-16T13:05:05Z

I have discussed this briefly with @roiser and @hageboeck in person.

There should not be any major issue. All CI tests passed, I will self merge.

Thanks again to @hageboeck for all the ideas on which this is based!

roiser · 2023-08-16T16:05:20Z

There is one situation where we would need to be careful, i.e. when switching the submodule to a another fork. E.g. I have checked out the MG5aMC with a branch of my (origin) fork

roiser@itscrd-a100:mg5amcnlo (roiser_gpucpp)$ pwd
/afs/cern.ch/work/r/roiser/sw/madgraph4gpu/MG5aMC/mg5amcnlo
roiser@itscrd-a100:mg5amcnlo (roiser_gpucpp)$ git remote -vv
origin	https://github.com/roiser/mg5amcnlo (fetch)
origin	https://github.com/roiser/mg5amcnlo (push)
upstream	https://github.com/mg5amcnlo/mg5amcnlo (fetch)
upstream	https://github.com/mg5amcnlo/mg5amcnlo (push)
roiser@itscrd-a100:mg5amcnlo (roiser_gpucpp)$ git branch -vv
  3.x           1a13d8f53 [upstream/3.x] merge with 2.9.16
  gpucpp        cb4771efc [upstream/gpucpp] In .gitignore, replace PLUGIN/CUDACPP_SA_OUTPUT by PLUGIN/CUDACPP*OUTPUT (the plugin will be installed as CUDACPP_OUTPUT) in the future
* roiser_gpucpp 74f91f0af [origin/gpucpp] force finalize for second exporter
roiser@itscrd-a100:mg5amcnlo (roiser_gpucpp)$

this will create a diff in the "outer" repository for the submodule link

roiser@itscrd-a100:MG5aMC (master)$ pwd
/afs/cern.ch/work/r/roiser/sw/madgraph4gpu/MG5aMC
roiser@itscrd-a100:MG5aMC (master)$ git diff
diff --git a/MG5aMC/mg5amcnlo b/MG5aMC/mg5amcnlo
index cb4771efc..74f91f0af 160000
--- a/MG5aMC/mg5amcnlo
+++ b/MG5aMC/mg5amcnlo
@@ -1 +1 @@
-Subproject commit cb4771efc507953c5ae03320ef5ee5496fabbff3
+Subproject commit 74f91f0affaceeeeb19e98dc862f379eebfe8598
roiser@itscrd-a100:MG5aMC (master)$

--> we should be careful to never commit/push this link e.g. via a PR, otherwise this wrong link will end up in the upstream repository pointing to (in this case) my fork for the submodule

valassi · 2023-08-16T16:30:37Z

Hi Stefan thanks. Ah ok now I understand what you mean. We had a misunderstanding between two cases.

You are right, if you checkout a different branch, and this branch has a different commit than gpucpp, then you risk committing the wrong commit hash. But if your branch has excatly the same content as gpucpp, then the commit is the same, and this should not change, can you check?
What I meant was excatly, that your branch has the same content as the commit hash. In that case, what I had understood from you was that we would risk changing the branch name in .gitmodules (not the commit hash in the submodule). In my understanding, this should NOT happen. Can you check?

…de new changes to cudacpp from MR madgraph5/madgraph4gpu#752)

Import new changes from madgraph5/madgraph4gpu#752

valassi · 2023-08-16T17:47:25Z

This is logically part oif #661, even if it is a slightly different issue. Step 2 will be a proper part of issue #661.

roiser · 2023-08-17T09:40:28Z

Hi Stefan thanks. Ah ok now I understand what you mean. We had a misunderstanding between two cases.

1. You are right, if you checkout a different branch, and this branch has a different commit than gpucpp, then you risk committing the wrong commit hash. But if your branch has excatly the same content as gpucpp, then the commit is the same, and this should not change, can you check?

yes you are right, if the two branches "upstream" and "origin" are at the same stage the commit hash is also the same, so there won't be a problem

The only possible problem left I see is if they diverge then we should understand if one does two PRs one for the submodule and one for the commit hash outside or we "adjust" the commit hash in a different way once the submodule PR has been merged.

2. What I meant was excatly, that your branch has the same content as the commit hash. In that case, what I had understood from you was that we would risk changing the branch name in .gitmodules (not the commit hash in the submodule). In my understanding, this should NOT happen. Can you check?

no that's ok, no problem here.
thanks Stefan

valassi · 2023-08-17T09:52:33Z

Hi, good, thanks for checking.

The only possible problem left I see is if they diverge then we should understand if one does two PRs one for the submodule and one for the commit hash outside or we "adjust" the commit hash in a different way once the submodule PR has been merged.

Hm I think that unfortunately we will need always two MRs anyway, one on madgraph4gpu and one on gpucpp. These are two different repos, I do not think we can combine change sto two repos in a single MR?

If indeed we need two MRs. then I guess it is better to first merge mg5amcnlo and only later merge madgraph4gpu, as the latter contains the hash to the former, not viceversa. This is almost exactly what I had with my commit.GIT anyway. Sometimes I was making changes in mg5amcnlo gpucpp in my fork and create a MR (so the hash would be in my fork anyway) and change madgraph4gpu even if that commit of gpucpp was not yet merged. But we should try to avoid it.

So I guess, ideally, first push/merge on mg5amcnlo gpucpp so that the commit hash is there, and then push/merge the reference to that hash in madgraph4gpu. Then sometimes there can be exceptions/deviations...

valassi added 12 commits August 15, 2023 15:26

[submod] add a git submodule mg5amcnlo in the top level MG5aMC

f78315b

mkdir <top>/MG5aMC cd <top>/MG5aMC git submodule add https://github.com/mg5amcnlo/mg5amcnlo

[submod] switch to branch gpucpp for mg5amcnlo

e308c38

cd <top> git submodule set-branch --branch gpucpp MG5AMC/mg5amcnlo (NB: the module name is MG5AMC/mg5amcnlo, you should not run this command with argument mg5amcnlo from directory MG5AMC) (NB: 'git config -l' shows no difference)

[submod] update module MG5aMC/mg5amcnlo to use the HEAD of gpucpp

b145ba4

git submodule update --remote Submodule path 'MG5aMC/mg5amcnlo': checked out '941832b9b03213da8ab994427add396673820799'

[submod] modify CODEGEN/generateAndCompare.sh to use MG5AMC_HOME from…

47cc558

… the mg5amcnlo submodule in this repo

[submod] in CODEGEN/generateAndCompare.sh, move code-generated direct…

d310065

…ories to $MG5AMC_HOME/../TMPOUT

[submod] add TMPOUT to MG5aMC/.gitignore

9a1e9db

[submod] in CODEGEN/generateAndCompare.sh, add back the cleanup of mg…

a060a17

…5amcnlo/PLUGINS after code generation

[submod] regenerate ggtt.sa - no change except for codegen log

195e07c

[submod] in CODEGEN/generateAndCompare.sh, remove commit.GIT and bran…

0fb5452

…ch.GIT

[submod] regenerate all other 6 processes sa - no change

b198086

valassi requested review from roiser, hageboeck and oliviermattelaer August 16, 2023 05:43

valassi self-assigned this Aug 16, 2023

valassi added 7 commits August 16, 2023 11:15

[submod] regenerate the other 7 processes mad (and add plugin_run_car…

36d5f66

…d for all of them)

[submod] in CODEGEN, configure renaming of CUDACCP_SA_OUTPUT as CUDAC…

7b68beb

…PP_OUTPUT when copying to mg5amcnlo

[submod] upgrade mg5amcnlo (use CUDACPP*OUTPUT) in .gitignore

f44100f

[submod] regenerate ggtt.sa after renaming plugin as CUDACPP_OUTPUT -…

b3ae790

… all ok, no change except log

[submod] regenerate ggtt.mad after renaming plugin as CUDACPP_OUTPUT …

2b0097d

…- all ok, no change except log

[submod] regenerate all other 6 sa after renaming plugin as CUDACPP_O…

9010836

…UTPUT - all ok, no change except log

[submod] ** COMPLETE SUBMOD ** regenerate all other 7 mad after renam…

e33fe84

…ing plugin as CUDACPP_OUTPUT - all ok, no change except log

valassi force-pushed the submod branch from ff106ef to e33fe84 Compare August 16, 2023 09:19

valassi merged commit bd255c0 into madgraph5:master Aug 16, 2023
21 checks passed

valassi added a commit to valassi/mg5amcnlo_cudacpp that referenced this pull request Aug 16, 2023

Merge remote-tracking branch 'glav/plugin' into cudacpp_plugin (inclu…

ed3452d

…de new changes to cudacpp from MR madgraph5/madgraph4gpu#752)

valassi mentioned this pull request Aug 16, 2023

Import new changes from madgraph5/madgraph4gpu#752 mg5amcnlo/mg5amcnlo_cudacpp#2

Merged

valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this pull request Aug 16, 2023

Merge pull request #2 from valassi/cudacpp_plugin

eddf774

Import new changes from madgraph5/madgraph4gpu#752

valassi mentioned this pull request Aug 16, 2023

upload plugins upstream to mg5amcnlo #661

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include mg5amcnlo in madgraph4gpu as a git submodule #752

Include mg5amcnlo in madgraph4gpu as a git submodule #752

valassi commented Aug 16, 2023 •

edited

Loading

valassi commented Aug 16, 2023

roiser commented Aug 16, 2023 •

edited

Loading

valassi commented Aug 16, 2023

valassi commented Aug 16, 2023

roiser commented Aug 17, 2023 •

edited

Loading

valassi commented Aug 17, 2023

Include mg5amcnlo in madgraph4gpu as a git submodule #752

Include mg5amcnlo in madgraph4gpu as a git submodule #752

Conversation

valassi commented Aug 16, 2023 • edited Loading

valassi commented Aug 16, 2023

roiser commented Aug 16, 2023 • edited Loading

valassi commented Aug 16, 2023

valassi commented Aug 16, 2023

roiser commented Aug 17, 2023 • edited Loading

valassi commented Aug 17, 2023

valassi commented Aug 16, 2023 •

edited

Loading

roiser commented Aug 16, 2023 •

edited

Loading

roiser commented Aug 17, 2023 •

edited

Loading