Skip to content

[WIP] GPU Abstraction to also target HIP in cudacpp impementation & Profiling infrastructure#718

Closed
Jooorgen wants to merge 0 commit into
madgraph5:masterfrom
Jooorgen:master
Closed

[WIP] GPU Abstraction to also target HIP in cudacpp impementation & Profiling infrastructure#718
Jooorgen wants to merge 0 commit into
madgraph5:masterfrom
Jooorgen:master

Conversation

@Jooorgen
Copy link
Copy Markdown
Member

Current work from the GPU abstraction I have been working on. Also includes the profiling infrastructure, but bear in mind the SYCL portion is not working as intended currently. Need some documentation from Nathan on how to compile it first.

Just ran it successfully on LUMI-G with MI250x GPUs. On a sidenote I seem to have an issue running on the GPUs we got from LHCb, but on LUMI it apperantly runs fine.

LHCb development GPUs (MI250x):

[jteig@n4051701 P1_epem_mupmum]$ ./gcheck.exe 32 32 10

ERROR! assertGpu: 'no ROCm-capable device is detected' (100) in ./GpuRuntime.h:62

gcheck.exe: ./GpuRuntime.h:21: void assertGpu(hipError_t, const char *, int, bool): Assertion `code == gpuSuccess' failed.

Aborted (core dumped)

VS

LUMI-G GPUs (MI250x):

teigjorg@nid005027:~/madgraph4gpu/epochX/cudacpp/ee_mumu.mad/SubProcesses/P1_epem_mupmum> ./gcheck.exe 32 32 10

..........

teigjorg@nid005027:~/madgraph4gpu/epochX/cudacpp/ee_mumu.mad/SubProcesses/P1_epem_mupmum>

@valassi
Copy link
Copy Markdown
Member

valassi commented Jun 21, 2023

Very good @Jooorgen thanks a lot! Will have a look next week.

Looking at this as a MR now - I guess we consider AMD still 'experimental', but one day we could set up an AMD GPU CI? @hageboeck any suggestion? Thanks

@roiser
Copy link
Copy Markdown
Member

roiser commented Jun 22, 2023

Very good @Jooorgen thanks a lot! Will have a look next week.

Looking at this as a MR now - I guess we consider AMD still 'experimental', but one day we could set up an AMD GPU CI? @hageboeck any suggestion? Thanks

For what concerns a CI I believe the best way forward would be to run them inside a container on HPCs, we don't have any machine on premise unfortunately.

@hageboeck
Copy link
Copy Markdown
Member

Very good @Jooorgen thanks a lot! Will have a look next week.
Looking at this as a MR now - I guess we consider AMD still 'experimental', but one day we could set up an AMD GPU CI? @hageboeck any suggestion? Thanks

For what concerns a CI I believe the best way forward would be to run them inside a container on HPCs, we don't have any machine on premise unfortunately.

Agreed. I don't think we will be able to implement an "immediate" check, though, i.e. run it on every PR. A nightly check might be easier to do.

@valassi
Copy link
Copy Markdown
Member

valassi commented Jul 13, 2023

Hi @Jooorgen as discused this looks very good! :-)

A few issues we worked on together are in my branch
https://github.com/valassi/madgraph4gpu/tree/jthip

Note, I have also just merged to master and to jthip a small fix for the issue we saw while working together (codegen failing on itscrd90 and not on itscrd80) ie bug #721.

I suggest you merge valassi/jthip and move over from there.

A few other minor issues/observations from our discussion

  • I would move GpuAbstraction.h as a file (nt a symlink) to src and remove the file from SubProcesses and the symlink in P1 (provided this is a file needed also in src? otherwise keep it in Subprocess with symlink in P1 and remove it from src)
  • Interesting that "<<<" works also in HIP as-is. You could have avoided implementing the launchKernel abstraction, if it was only for HIP and CUDA. On the other hand, if one day we go to SYCL, we might need this anyway? So keep it as you did.
  • Nice that you replaced _CUDACC with MGONGPUCPP_GPUIMPL! But I am not sure if you need to define those as CUDACC and HIPCC. Maybe "cuda" and "hip" would be enough? I am slightly confused as I think the cpp preprocessor does not parse those twice
  • In the gpuDeviceReset there is no checkCuda now, but maybe this is not needed?

Thanks again!

Andrea

@valassi
Copy link
Copy Markdown
Member

valassi commented Jul 13, 2023

NB This is related to issue #311 (it implements the first part of it namely code and builds - second part will be to make tests, maybe better open a separate issue?)

@valassi
Copy link
Copy Markdown
Member

valassi commented Jul 18, 2023

Hi @Jooorgen I have completed the work we were doing together this afternoon

  • the CODEGEN is completed and stable, I checked that ggttgg.mad builds ok an druns simple tests
  • I have now regenerated all 7 processes mad and all 7 processes sa, and am rerunning the tests

Please merge this into your branch
https://github.com/valassi/madgraph4gpu/tree/jthip

As discussed, we may consider having a MR from a simpler branch rather than from the full master, But if you say that the files in profile are needed, its ok as is I guess.

Anyway it would be good t hav ethe CI executed, and passing, atr least on CPU and NVidia GPU (as we have no CI on HIP)

Thanks!
Andrea

@valassi
Copy link
Copy Markdown
Member

valassi commented Jul 25, 2023

Hi @Jooorgen I have merged upstream/master (including my FPE and namespace changes) into my
https://github.com/valassi/madgraph4gpu/tree/jthip
(where I had added your latest changes).

Please merge this into yours and lets see... it works for CUDA, hopefully also for HIP?

Thanks
Andrea

@valassi
Copy link
Copy Markdown
Member

valassi commented Jul 26, 2023

Hi @Jooorgen I have also merged my very upstream/master (including my FPE changes for icx from MR #737) into my
https://github.com/valassi/madgraph4gpu/tree/jthip
(where I had added your latest changes).

Please merge this into yours and lets see... it works for CUDA, hopefully also for HIP?

Thanks
Andrea

valassi added a commit to valassi/madgraph4gpu that referenced this pull request Jan 25, 2024
I am working now in branch valassi/jthip24.
This is my first commit over joorgen/gpu_abstraction branch (also known as valassi/jthip),
as of commit 229ffeb (Tue Aug 15 11:33:01 2023 +0200).

This branch contains fewer features than jooorgen/master (PR madgraph5#718),
but it is more advanced than joorgen/gpu_abstraction_only (PR madgraph5#774).
I will probably need some of the commits here to fix PR madgraph5#774 in branch valassi/jt774.

I regenerate the ten processes as follows:
  ./CODEGEN/generateAndCompare.sh ee_mumu --mad
  ./CODEGEN/generateAndCompare.sh gg_tt --mad
  ./CODEGEN/generateAndCompare.sh gg_ttg --mad
  ./CODEGEN/generateAndCompare.sh gg_ttgg --mad
  ./CODEGEN/generateAndCompare.sh gg_ttggg --mad
  ./CODEGEN/generateAndCompare.sh ee_mumu
  ./CODEGEN/generateAndCompare.sh gg_tt
  ./CODEGEN/generateAndCompare.sh gg_ttg
  ./CODEGEN/generateAndCompare.sh gg_ttgg
  ./CODEGEN/generateAndCompare.sh gg_ttggg
These are the five processes that would get conflicts when I merge upstream/master here,
so I guess that these are the only ten processes touched in this branch.
The fact that I can regenerate them and there are no real differences
(except for irrelevant stuff like me5_configuration.txt, aloha_file.inc, py3_model.pkl)
shows that ALL IMPORTANT CHANGES BY JORGEN HERE ARE IN THE CODEGEN.

I can therefore merge upstream/master, fix conflicts in CODEGEN and regenerate.
@Jooorgen Jooorgen closed this May 4, 2026
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.

4 participants