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

master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp #121

Merged
merged 30 commits into from
Sep 3, 2024

Conversation

valassi
Copy link
Contributor

@valassi valassi commented Jul 19, 2024

Hi @oliviermattelaer this is a WIP MR into gpucpp of my nb_warp_used and of everything else I am using against master_june24 in my (madgraph5/madgraph4gpu#882).

The nb_warp_used fixes are described here madgraph5/madgraph4gpu#885 (comment)

NB: as discussed in madgraph5/madgraph4gpu#886, gpucpp_june24, with or without my changes, does NOT work for me with master_june24 (my modified version, but I am pretty sure that the problem was pre-existing). This is why here I am NOT merging gpucpp_june24, I am only merging a SUBSET of that (what was originally used in master_june24, plus my changes, see https://github.com/valassi/mg5amcnlo/commits/valassi_gpucpp_june24/

I keep this in WIP for the moment. I would only merge this when we agree to merge my madgraph5/madgraph4gpu#882 into master (as master is meant to use the head of gpucpp... I do not want to 'pollute' gpucpp before!).

I also filed PR #120 against gpucpp_june24, but as mentioned in madgraph5/madgraph4gpu#886, I am unable to use gpucpp_june24 and this is NOT what was used in master_june24 anyway.

Thanks Andrea

oliviermattelaer and others added 28 commits September 14, 2023 10:18
…section (and wrap not correctly ported everywhere
…pucpp_june24 (eef200f)

This is needed to allow the merge of the following two branches of madgraph4gpu:
- master 6ba6bae30 (which uses commit 1e2aa4b i.e. the head of origin/gpucpp)
- master_june24 309a48654 (which uses eef200f that is currently a standalone commit)
@valassi valassi requested a review from oliviermattelaer July 19, 2024 12:36
@valassi valassi self-assigned this Jul 19, 2024
@valassi valassi marked this pull request as ready for review August 21, 2024 17:49
@valassi valassi changed the title WIP: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp Aug 21, 2024
@valassi
Copy link
Contributor Author

valassi commented Aug 21, 2024

Hi @oliviermattelaer this is a preliminary step for merging the master_june24 work of madgraph5/madgraph4gpu#882

Can you please review and approve?

Note, this is related to madgraph5/madgraph4gpu#886: in practice, what is here my valassi_gpucpp_june24 gets merged into gpucpp, while we forget about gpucpp_june24.

Thanks!
Andrea

@valassi valassi changed the title master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp URGENT master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp Aug 21, 2024
@valassi
Copy link
Contributor Author

valassi commented Aug 21, 2024

PS @oliviermattelaer PLEASE DO NOT SQUASH in case you merge this, thanks :-)

…g5amcnlo#132) into valassi_gpucpp_june24

Fix conflicts: madgraph/various/banner.py (keep a debug printout in get_value_from_include)
@valassi
Copy link
Contributor Author

valassi commented Sep 1, 2024

Hi @oliviermattelaer I just merged th elatest gpucpp (including your gpucpp_fixci #132) into my valassi_gpucpp_june24.

I will update also my madgraph5/madgraph4gpu#882 in madgraph4gpu to use this one.

(Then I will probably comment on your madgraph5/madgraph4gpu#981 that 981 becomes irrelevant)

@valassi
Copy link
Contributor Author

valassi commented Sep 1, 2024

Hi @oliviermattelaer this now includes everything we need for june24. Can you have a look and approve? Thanks
Andrea
PS Again please do not squash this...

@oliviermattelaer
Copy link
Contributor

Looks like they are conflict to fix here.
Additionally don't you want to use nb_warp_used? in the file dsample.f
Or do you prefere anoter PR for that?

Olivier

@@ -1738,7 +1738,25 @@ def get_pdf_lines(self, matrix_element, ninitial, subproc_group = False, vector=
if vector:
pdf_definition_lines_vec = ""
pdf_data_lines_vec = ""
pdf_lines = " DO iVEC=1,VECSIZE_USED\n"
pdf_lines = """ NB_WARP_USED = VECSIZE_USED / WARP_SIZE
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @oliviermattelaer about your question: I am doing nb_warp_used in various places. This is one example here.

But you are right that there may be more. (See madgraph5/madgraph4gpu#983)

Ok I will check dsample.f more in detail...

@valassi
Copy link
Contributor Author

valassi commented Sep 2, 2024

Hi Olivier thanks

Looks like they are conflict to fix here.

Hm? No I do not see conflicts, which conflicts are you talking about?

Additionally don't you want to use nb_warp_used? in the file dsample.f
Or do you prefere anoter PR for that?

No I wanted to fix this here.

I am fixing it in various places.
See https://github.com/mg5amcnlo/mg5amcnlo/pull/121/files#r1740773122

I thought that this was enough because tests were passing.
But you are right that I may be missing some and that these go undetected.
Ok then lets put this back to WIP :-(

Andrea

@valassi valassi changed the title URGENT master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp WIP master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp Sep 2, 2024
@oliviermattelaer
Copy link
Contributor

Screenshot 2024-09-02 at 13 56 21

Ok looks like this is another subtelty of github, that I had to understand... But this is only because the button was on "rebase" if I move it to "merge" then github does not complain about conflict anymore ...
Sorry for the confusion

@valassi valassi changed the title WIP master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp master_june24 completion: nb_warp_used (and a SUBset of gpucpp_june24) into gpucpp Sep 2, 2024
@valassi
Copy link
Contributor Author

valassi commented Sep 2, 2024

Additionally don't you want to use nb_warp_used? in the file dsample.f

Hi @oliviermattelaer again. I had a look at this, see the comments in madgraph5/madgraph4gpu#983. In my opinion there is nothing else to be done in dsample.f, but I may be missing something. Can you have a look yourself and comment in 983 please?

Then if we agree that there is nothing else to do , I think that we can start merging this.

Thanks Andrea

@valassi
Copy link
Contributor Author

valassi commented Sep 3, 2024

Hi @oliviermattelaer again. I had a look at this, see the comments in madgraph5/madgraph4gpu#983. In my opinion there is nothing else to be done in dsample.f, but I may be missing something. Can you have a look yourself and comment in 983 please?

Hi @oliviermattelaer thanks for having a look! You confirmed here madgraph5/madgraph4gpu#983 (comment) that there is nothing to do for 983, so I consider that this is good to go.

So, I AM MERGING THIS! Starting the full june24 merge into master+gpucpp...

@valassi valassi merged commit a696c93 into mg5amcnlo:gpucpp Sep 3, 2024
valassi added a commit to valassi/mg5amcnlo that referenced this pull request Sep 3, 2024
… into ghav_gpucpp_june24

NB: at this point, the remaining changes in this branch w.r.t. gpucpp are only a few commits by Olivier (which may be elsewhere like360)

git log --oneline gpucpp..HEAD
718f59a0e (HEAD -> ghav_gpucpp_june24) Merge branch 'gpucpp' (including june24 nb_warp_used via mg5amcnlo#121) into ghav_gpucpp_june24
268cee3 (ghav/ghav_gpucpp_june24) Merge branch 'valassi_gpucpp_june24' into ghav_gpucpp_june24
def7fa6 sync gpuccp_june24 with gpucpp
37d1871 fix change in cpp template within the test (spaces)
0e5e483 fixing pythia8 iotest
849194f fix some tests (template changed)
1a9e6dd fixing an issue in the fortran side if no vector size
e2e7900 add a possibility to have multiple type of factorization (no actual code change at this stage)
942dab7 secure the code
45f8410 (origin/gpucpp_fix_scan_vector) Merge branch 'gpucpp_june24' into gpucpp_fix_scan_vector
0959edf Merge branch 'gpucpp_wrap' into gpucpp_june24
63d2d39 (origin/gpucpp_wrap) remove useless file + debug statement
ecb0a03 add gpu in the autocompletion for group_subproceses
82f078b change makefile to recompile code as needed if vector size is changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants