ralna / spral

Sparse Parallel Robust Algorithms Library
https://ralna.github.io/spral/
Other
104 stars 27 forks source link

omp_get_nested routine deprecated #124

Closed amontoison closed 1 year ago

amontoison commented 1 year ago

In GALAHAD, we have the following warning if we compile with Intel compilers:

OMP: Info #269: OMP_NESTED variable deprecated, please use OMP_MAX_ACTIVE_LEVELS instead.
OMP: Info #277: omp_get_nested routine deprecated, please use omp_get_max_active_levels instead.

https://www.openmp.org/spec-html/5.0/openmpsu120.html https://www.openmp.org/spec-html/5.0/openmpsu126.html

jfowkes commented 1 year ago

@mjacobse any thoughts on how best to fix these warnings? The issue with using OMP_MAX_ACTIVE_LEVELS is that one has to explicitly specify the number of levels.

Ideally I would want to get rid of the need to set the three OMP variables:

export OMP_CANCELLATION=TRUE
export OMP_NESTED=TRUE
export OMP_PROC_BIND=TRUE

before running the code as this is tedious and not user-friendly. I fail to understand why the OpenMP standard does not have a programmatic way of setting these in the code.

mjacobse commented 1 year ago

I am not really too familar with these advanced OpenMP features. As far as I can tell from the documentation, using omp_set_max_active_levels to set the max active levels to what is returned by omp_get_supported_active_levels would be equivalent to setting OMP_NESTED=TRUE.

For the nested setting, this already seems to be done programmatically anyways: https://github.com/ralna/spral/blob/4ec87975f8132debfb0f2da2c6e8740fa97132b0/src/ssids/ssids.f90#L1459-L1461 https://github.com/ralna/spral/blob/4ec87975f8132debfb0f2da2c6e8740fa97132b0/src/ssids/ssids.f90#L1467-L1469

I even think the second of these sections does nothing at all right now, since omp_set_nested already sets the max-active-levels-var ICV to something larger than 2. So I believe replacing both sections with just

 !$  user_settings%max_active_levels = omp_get_max_active_levels() 
 !$  if (user_settings%max_active_levels .lt. 2) call omp_set_max_active_levels(omp_get_supported_active_levels())

would be the non-deprecated, equivalent version. But just deleting the first section and keeping the second section should also work and not be deprecated.

jfowkes commented 1 year ago

Thanks @mjacobse, that's great news! I will create a PR to delete the first section as this seems cleanest.

I still find it deeply annoying that cancellation can only be enabled via the setting of an environment variable, who on the OpenMP standards committee actually thought that was a good idea?