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

Why do we need to compile with openmp flag? #758

Closed
oliviermattelaer opened this issue Aug 31, 2023 · 7 comments · Fixed by #874 or #900
Closed

Why do we need to compile with openmp flag? #758

oliviermattelaer opened this issue Aug 31, 2023 · 7 comments · Fixed by #874 or #900
Assignees

Comments

@oliviermattelaer
Copy link
Member

Should we remove the openmp flag completely?
or have a way to compile with it on request?

I had to change a makefile to compile on my mac (but will likely create issue with the CI)

valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 28, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 28, 2024
@valassi valassi self-assigned this Jun 28, 2024
@valassi
Copy link
Member

valassi commented Jun 28, 2024

Hi @oliviermattelaer good point.
(Sorry for the delay in replying).

I have implemented a simple mechanism whereby you can disable OpenMP if you externally set OMPFLAGS= as an empoty string

See cb67911

Conversely, if OMPFLAGS is set externally to something else, for the moment I prefer to ignore this and keep control.

valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 28, 2024
@oliviermattelaer
Copy link
Member Author

Perfect thanks,

I think that we should have an agreement on how we handle OpenMP by default from the madgraph5 interface.
(i.e. should I set OMPFLAGS= by default or not, should we have an entry in the run_card to control that /...)

Cheers,

Olivier

@valassi
Copy link
Member

valassi commented Jul 4, 2024

Perfect thanks,

I think that we should have an agreement on how we handle OpenMP by default from the madgraph5 interface. (i.e. should I set OMPFLAGS= by default or not, should we have an entry in the run_card to control that /...)

Cheers,

Olivier

Thanks a lot Olivier! As discussed in #874 (that I just merged), I reopen this so that we can discuss waht to do... assign it also to you so you remember to provide more suggestions

@valassi valassi reopened this Jul 4, 2024
@oliviermattelaer
Copy link
Member Author

I wanted to know yours and @roiser opinion

On my side, I would naively move for OMP OFF by default.
For SIMD case, they are no gain to use OMP.
For the GPU case, this is more blurr but the OMP but I also guess that having multi-process is better than OpenMP (helping to overlap cpu/gpu compuation between the thread)

@roiser
Copy link
Member

roiser commented Jul 4, 2024

Yes, having multiple processes hammering the GPU is better, but I'm not sure its really needed for the real world use case where we can control how many processes we want to run via the mg5_configuration.txt with nb_core. This is also what I partly (re)exercised when doing the A100/H100 comparison (slides on the indico of the last MG5 dev meeting).

@valassi
Copy link
Member

valassi commented Jul 5, 2024

I think that this is a complex issue that requires tests and optimizations. And the experiments are doing a lot of this on their own (and we may help them). We had introduced OMP because it is a one-liner way to use many threads, then people used it claiming it was our best thing, while it was a 5-minute game. I would disable OMP by default, mainly because it makes debugging easier. Then I would not remove the OMP code, which sometimes may be useful. So yes I vote to disable OMP by default. Let me know, I can implement that.

valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 10, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 10, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 10, 2024
@valassi valassi linked a pull request Jul 10, 2024 that will close this issue
@valassi
Copy link
Member

valassi commented Jul 10, 2024

Following yesterday's discussion I changed this: OpenMP is disabled by default now in PR #900 (pening Olivier's review).

It is enabled only if USEOPENMP=1. (This internally checks also if the O/S compiler etc allow this, therefore OMPFLAGS is not set externally but internally depending on USEOPENMP)

valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 12, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
…from fpe PR madgraph5#874 and omp PR madgraph5#900 for OpenMP issue madgraph5#758 - bldall and runTest succeds
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
…, disable OpenMP by default unless USEOPENMP=1
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
…ph5#758 (keep OMP disabled but add the option to enable it)
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jul 17, 2024
…annelid in MatrixElementKernelBase, and replace OMPFLAGS by USEOPENMP madgraph5#758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment