madgraph5 / madgraph4gpu

GPU development for the Madgraph5_aMC@NLO event generator software package
28 stars 33 forks source link

Further cleanup of makefiles after separating C++ and CUDA targets #841

Closed valassi closed 1 month ago

valassi commented 1 month ago

This is a followup to PR #798 built over Jorgen's changes, separating C++ and CUDA targets.

In this new situation, I found it appropriate to also clean up variables and object names, so that it is clear whether they refer to the C++ or CUDA/HIP build. I could have added this over #798 as the changes make no functional difference at all, but the lines of code that change are a lot and it's cleaner to keep it as a separate PR.

This is still in WIP, I will makr it for review after merging #798.

valassi commented 1 month ago

Hi @oliviermattelaer this is alos good to go for me.

I mark you as reviewer, but I suggest that you first approve #798 and I merge that one first, so that you can see the additional differences here. In this one essentially there are only additional makefile changes, changing the names of objects and executables (and of makefiles variables to make this all more consistent internally.

Some of the main examples:

These changes are possible (and appropriate I think) because after #798 we ONLY build EITHER cuda or hip or one of the cpp simd, we no longer mix/link any of these with one another. So I think that it's good to remove the old gcheck naming, but also good to keep a _cpp or _cuda or _hip suffix to understand immediately how these things were built and what they do. This is consistent with the madevent_cpp, madevent_cuda and madevent_hip naming convention we have for the madevent executables.

So again: wait before looking at these changes, lets get #798 merged first, Thanks! Andrea

valassi commented 1 month ago

Hi @oliviermattelaer thanks for approvin PR #798, which is now merged.

Can you please now review this additional PR #841? This contains a few additional cleanups of makefiles as discussed above. It is now much easier to review as you will only see the few additional changes (previously it also contained all of #798).

These are mainly name changes in build products (.o and .exe) and in makefile variables. It is possible (and proopriate I think) because now we cleanly build EITHER a gpu (cuda or hip) version OR a specific cpp version, we never link gpu with cpu anymore.

Thanks! Andrea

valassi commented 1 month ago

Thanks a lot, very good I will merge it