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

What to do with OMPI_MPI_THREAD_LEVEL env variable? #9332

Open
jsquyres opened this issue Aug 30, 2021 · 8 comments
Open

What to do with OMPI_MPI_THREAD_LEVEL env variable? #9332

jsquyres opened this issue Aug 30, 2021 · 8 comments

Comments

@jsquyres
Copy link
Member

This is an opening to a discussion about the OMPI_MPI_THREAD_LEVEL environment variable. There's a few issues here:

  1. This env variable was added a long time ago to MPI_Init() as a way for the OMPI developers to set the MPI thread level for apps that call MPI_Init() (not MPI_Init_thread()). This allowed us to the thread code in Open MPI without updating every single OMPI test to call MPI_Init_thread().
  2. We specifically did not add the same override to MPI_Init_thread() because the thought was that in this case, the app had asked for exactly what it wanted in the required param. Meaning: the env variable override was for legacy apps who still call MPI_INIT.
  3. Master commit babfd93 changes the behavior of MPI_Init_thread() to also obey the value from the OMPI_MPI_THREAD_LEVEL environment variable. This is a behavior change.
  4. That being said, it's possible we never documented OMPI_MPI_THREAD_LEVEL to end users. I honestly don't remember if it was intended to be a developer-only feature or whether we intended to allow end users to use this functionality.
  5. If we want to make this functionality available to the end user, we might consider adding this as an MCA param and treating it like all other MCA params (vs. a one-off env variable).
  6. Additionally, if it's to be used by both MPI_INIT and MPI_INIT_THREAD (and MPI sessions?) it should probably be implemented back in ompi_mpi_init() (does the Sessions prototype use ompi_mpi_init()?) -- not handled separately in both MPI_Init() and MPI_Init_thread().
  7. Reading MPI-4.0 description of MPI_INIT and MPI_INIT_THREAD, I'm not actually sure that the OMPI_MPI_THREAD_LEVEL env variable is actually compliant with the spec. It makes a few statements about how calling MPI_Init() is the same as calling MPI_Init_thread() with MPI_THREAD_SINGLE. This might affect whether we want to expose this "backdoor" functionality to end users.

I think a few discussions fall out of the above points:

  1. Do we want to keep the ability to override the thread level at run time?
  2. Which functions does this functionality apply to (MPI_INIT, MPI_INIT_THREAD, sessions functions, ...)?
  3. Do we expose this functionality to the end user, or is it intended to be developer-only?
  4. Should this remain an env variable, or should we move it to an MCA param?
@bosilca
Copy link
Member

bosilca commented Aug 30, 2021

In same cases modifying the source code of an application/library is not an option, and being able to alter the behavior of the MPI library remains a critical capability. Among the things users need to control, the thread level comes high, because this will allows the MPI library to handle things in a threaded way (async progress being one of these opportunities).

However, as noted in #9312 we have an issue if the user set the OMPI_MPI_THREAD_LEVEL in the way that collides with the application requested thread model. Thus, we need to be careful on the meaning of this variable, and instead of allowing it to force set the thread level we should only allow it to escalate the requested thread level (using the thread level defined by the MPI standard).

And as usual 👍 on more documentation.

@gpaulsen
Copy link
Member

In the past our team has also decreased the thread level of an MPI_THREAD_MULTIPLE application, when we wanted to test the single threaded approach.

Remember the thread level specified to MPI_Init_thread is not a guarantee, but a request by the application. The MPI implementation is free to provide a lower level of threading. Users are then supposed to see what thread level they received by checking the last argument given to MPI_Init_thread. The application can then decide how to handle the level of threading provided by the MPI implementation.

@jsquyres
Copy link
Member Author

We talked about this on the 31 Aug 2021 Open MPI webex. Conclusions that we came to:

  1. For v4.0.x and v4.1.x, IBM indicated that they would like to have the capability of an environment variable to override a compiled app's MPI thread level. To ensure that no one is using the old environment variable with a 4.0.x or 4.1.x application and unexpectedly has the env variable change the behavior of MPI_Init_thread(), the suggestion was made to:
    • Leave the old env variable in MPI_Init()
    • Add a new env variable to MPI_Init() and MPI_Init_thread() (or perhaps even ompi_mpi_init()) that performs the same function as OMPI_MPI_THREAD_LEVEL.
      This gives IBM the functionality that they want, but also guarantees that no one will use the old env variable name and be surprised of new behavior in MPI_Init_thread().
  2. For master and v5.0.x, convert the env variable to be a proper MCA parameter, and ensure that it works across MPI_Init, MPI_Init_thread, and can also be made to work in the MPI Sessions prototype (e.g., implementing it in ompi_mpi_init() might just cover all relevant cases).
  3. In all cases, there value passed in via the env variable/MCA param will be treated as app's requested thread level. Open MPI still reserves the right to return a different value in the provided thread level.

@bosilca
Copy link
Member

bosilca commented Aug 31, 2021

I checked the sessions requirements, and I think this will affect the sessions in the same way it affects the MPI_COMM_WORLD model, aka. by enforcing a selectable thread-level.

@gpaulsen gpaulsen self-assigned this Sep 2, 2021
@gpaulsen
Copy link
Member

I started to implement this, this morning as an mca parameter. Unfortunately the mca system setup needs to know what the thread level is beforehand since it passes that info to various components which behave differently based on the thread level.

I will just PR using an env var so that I don't have to touch the mca infrastructure.

@jsquyres
Copy link
Member Author

jsquyres commented Oct 7, 2021

@gpaulsen Any progress on this?

Does this need to be done for v5.0.0? I.e., are there backwards compatibility issues with releasing it after v5.0.0?

@awlauria
Copy link
Contributor

awlauria commented Oct 14, 2021

master: #9302
v5.0.x: #9313

I thought we decided to leave these changes in master/v5.0.x as-is, and make a different change for v4.0.x/v4.1.x. Is that accurate @jsquyres @gpaulsen ?

@jsquyres
Copy link
Member Author

jsquyres commented Nov 1, 2021

I agree; I thought that IBM was interested in back-porting to at least v4.1.x so that you guys could have parity with the community release...?

@gpaulsen gpaulsen removed their assignment Aug 22, 2023
@jsquyres jsquyres modified the milestones: v5.0.0, v5.0.1 Oct 30, 2023
@janjust janjust modified the milestones: v5.0.1, v5.0.2 Jan 8, 2024
@jsquyres jsquyres modified the milestones: v5.0.2, v5.0.3 Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants