-
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
Improvements to complete PRs 979 and 980 #984
Conversation
…ll' instead of 'make cleanavx' to clean the build (these are identical)
… up P* cleanup; remove cleanavx which is identical to cleanall (launch_plugin.py has been changed accordingly)
…efile, split clean into cleanSource+clean and move cleanavxs at the end via %(additional_clean)
…k cleanSource to speed up P* cleanup Also remove cleanavx which is identical to cleanall (launch_plugin.py has been changed accordingly) Add some comments in Olivier's write_source_makefile
…l commit message later (regenerating the patch changes nothing)
…g changes: I just want to mark that Source/makefile is no longer there) The only files that still need to be patched are - 2 in patch.common: Source/genps.inc, SubProcesses/makefile - 3 in patch.P1: auto_dsig1.f, driver.f, matrix1.f ./CODEGEN/generateAndCompare.sh gg_tt --mad --nopatch git diff --no-ext-diff -R gg_tt.mad/Source/genps.inc gg_tt.mad/SubProcesses/makefile > 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/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
Hi Andrea, I'm completely confused here. I know that I do not yet fully understand github and this is likely the issue here. Is their an issue in the way I created those branches (or in the way you handle those? --I guess the issue is on my side--) I will try to create such diff manually but if you have a trick or an understanding why the diff in such PR are so bad? it could be usefull |
Ok even with the "diff" that I want, looks like you are merging something else at the same time. Otherwise in launch_plugin.py, you replace So I would propose to have
Now i would say that cleanall can be
but I might miss a point here, so maybe they are an advantage to keep your current definition |
…_fixci): fix my bug, define additional_clean in the second place where it is needed in export_v4.py
Hi @oliviermattelaer I am sorry this is confusing (This is exactly why I was saying that using submodules is nice but can be confusing). There are two parts here, what I added on top of mg5amcnlo and what I added on top of madgraph4gpu. (1) For mg5amcnlo, this is now into your mg5amcnlo/mg5amcnlo#132 (There are still issues to solve there, but hopefully not in my part) (2) For madgraph4gpu, let me explain. I think the problem is that your testsuite_only_fixed did not include the latest master. Now I have modified that by merging master into it (well I included it into testsuite_only, and then the latter into testsuite_only_fixed). So now that you modified this PR to target testsuite_only_fixed you should be able to see the differences. The commits are
And the diffs are here, excluding regenerated code
In practice:
Otherwise this is essentially identical to your 980... Does this clarify? If you agree, I would
NB: in mg5amcnlo/mg5amcnlo#132 I think there are still some acceptance tests failing, but these are in your code/tests and I have no idea how to look at them... |
Sorry, my mistake again... I did not understand what you meant, now I see it Before I changed it, the two targets were (this is current master)
I thought they are identical, they are not. The difference is that calling 'cleanall' ALSO cleans Source, while cleanavx does not. I do not remember why 'cleanavx' got there, I thought I had added it and forgotten it by mistake, but maybe you added it for this reason. Anyway: the part that is really slow, both in the cleanup and in the build, is the P* subdirectories. Deleting and recreating the objects in Source is very fast compared to that. For extra safety I would actually remove those and recreate them. Or you would prefer to avoid that?
Anyway, to be clear. It was not on purpose in the sense that I made a mistake and I had not noticed. That said, now that I did notice, I actually think that it is a small overkill and I would keep that. Rephrasing: are you sure yourself that in Source there are things that do not depend on vector.inc and need no recompilation when vector.inc changes? I see for instance that you added a dependency of libmodel on vector.inc, so I guess it must be recompiled? (By the way, I noticed a strange error that I will report with 'vector.inc missing for lepton pdf', which is exactly related to vector.inc and libmodel). |
…'make cleanavxs' target as suggested by Olivier
Hi @oliviermattelaer as discussed offline, I merge this into your branch for #980. Then I will merge #979 and #980 into master. Thanks Andrea |
…, madgraph5#980, madgraph5#984 patches for the new CI and Source/makefile) into june24 Fix conflicts: - MG5aMC/mg5amcnlo (keep the current june24 version 4ef15cab1 i.e. current valassi_gpucpp_june24) - epochX/cudacpp/gg_tt.mad/bin/internal/banner.py (keep a debug printout)
Hi @oliviermattelaer these are my improvements to complete PRs #979 and #980.
If you agree with this, I can merge the whole lot.
This depends on mg5amcnlo/mg5amcnlo#135. Please review that one first, or together with this.
Can you review please? THanks Andrea