Skip to content

Conversation

@pscicluna
Copy link

Mie calculus is pretty slow, and sometimes it can be useful to execute it in parallel. I have added OpenMP directives in appropriate places in the code - in a trade-off between efficiency and expected number of times it is used, the code parallelises over grain sizes.

…ill just use all available CPU cores. But first have to figure out how I'm supposed to modify the configure script to inject an omp option. After that, will want to make num_threads a runtime variable that is accepted as input by the code.
…allel mode right now. Next step: allow user choice of number of threads
…ight have to look into rearranging the code slightly to improve this if this is the source of problems by dumping everything into large arrays and then collapsing them outside the loop.
Copy link
Contributor

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! A short comment/question below.

real(dp) :: wav_min, wav_max


integer :: n_threads=1 !needed for omp parallel version
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, is it not possible to set the number of threads using the OMP_NUM_THREADS environment variable, which would mean no needing to define this variable?

Copy link
Author

Choose a reason for hiding this comment

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

In principle yes, but, my experience with the environment variables has been iffy at best, with them not always creating the desired behaviour, while omp_set_num_threads() always works. As a result, I tend to prefer the omp directive.

However, omp_set_num_threads() will override OMP_NUM_THREADS if it exists, which can be good or bad - a user needs to remember to set two things if they're using more than one package, but they also have the flexibility to easily change behaviour independently.

In short, it should work either way, but this way is explicit.

Copy link
Author

Choose a reason for hiding this comment

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

I wanted to come back to this - it wasn't clear to me from your comment if you wanted me to make a change or just explain the rationale behind the implementation. Given that you haven't resolved the comment I'm assuming you want the change implemented, but it would be good to clarify that before I make more updates if you are interested in merging the PR.

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.

2 participants