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

omp_get_nested routine deprecated #124

Closed
amontoison opened this issue Jul 20, 2023 · 3 comments · Fixed by #125
Closed

omp_get_nested routine deprecated #124

amontoison opened this issue Jul 20, 2023 · 3 comments · Fixed by #125

Comments

@amontoison
Copy link
Member

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
Copy link
Contributor

jfowkes commented Jul 20, 2023

@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
Copy link
Collaborator

mjacobse commented Aug 2, 2023

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:

spral/src/ssids/ssids.f90

Lines 1459 to 1461 in 4ec8797

!$ ! must have nested enabled
!$ user_settings%nested = omp_get_nested()
!$ if (.not. user_settings%nested) call omp_set_nested(.true.)

spral/src/ssids/ssids.f90

Lines 1467 to 1469 in 4ec8797

!$ ! we will need at least 2 active levels
!$ user_settings%max_active_levels = omp_get_max_active_levels()
!$ if (user_settings%max_active_levels .lt. 2) call omp_set_max_active_levels(2)

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
Copy link
Contributor

jfowkes commented Aug 2, 2023

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?

@jfowkes jfowkes linked a pull request Aug 2, 2023 that will close this issue
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 a pull request may close this issue.

3 participants