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

How to handle component threads within CIME #67

Closed
douglasjacobsen opened this issue Feb 22, 2016 · 15 comments
Closed

How to handle component threads within CIME #67

douglasjacobsen opened this issue Feb 22, 2016 · 15 comments

Comments

@douglasjacobsen
Copy link
Member

I asked this question last week during a telecon, but wanted to ask it again here just to document the answer.

How are threads handled on a per-component basis within CIME?

Does the driver set the correct number of threads? or are component expected to set the number of threads through some pre-defined information (i.e. whatever exists in env_mach_pes.xml)?

Also, what is the preferred way to get / set the number of threads a component should be using?

@douglasjacobsen
Copy link
Member Author

Based on the call from last week, I think this is the correct way to do it, but if someone knows of a better way, please post it.

call seq_comm_setptrs( COMPID, nthreads=comp_nThreads)
call seq_comm_setnthreads(comp_nThreads)

@rljacob
Copy link
Member

rljacob commented Feb 29, 2016

Apparently there was a change between cesm1.2 and cesm1.3. In 1.2, the driver set the number of threads for each component. In 1.3, that was left up to each component and CIME preserved the 1.3 behavior.

Does anyone remember why it was changed between 1.2 and 1.3?

@jedwards4b
Copy link
Contributor

I don't think that it was intentionally changed and as far as I know it should be as in 1.2 (did it actually work that way?) I've had it on my todo list for a while to fix this again but it involves coordination with all the components.

@worleyph
Copy link
Contributor

I may have provided misinformation to @rljacob . It clearly is not working as expected currently, and I believe that it used to. However, looking again, I see

         if (drv_threading) call seq_comm_setnthreads(comp(1)%nthreads_compid)
 ...
         if (drv_threading) call seq_comm_setnthreads(nthreads_GLOID)

in component_mod.F90 in CIME.

I thought that I compared cesm1.2 and cesm1.3 and saw a change in implementation. I'll go back and see what confused me, or try to clarify this. Perhaps drv_threading is no longer on by default?

@worleyph
Copy link
Contributor

Okay, my confusion was from looking at drvseq5_0_12, which was used in cesm1_3_beta10 (the ACME starting point). The only thread setting commands in ccsm_comp_mod.F90 are of the form:

      if (drv_threading) call seq_comm_setnthreads(nthreads_CPLID)
 ...
       if (drv_threading) call seq_comm_setnthreads(nthreads_GLOID)

(and sometimes these are not matched up correctly as well?)

However, this tag had already moved the component-specific thread logic into component_mod.F90, and I did not look there.

So, the conversation within ACME is that some people considered it cleaner to have the driver control how many threads each component uses. This appears to continue to be the CESM and CIME design as well? (This is unclear from some of the comments above.) If so, the next step is to find out how to get it to work as expected.

@jedwards4b
Copy link
Contributor

I agree that the driver should tell each component the number of threads to use and I believe that the driver code to do this is in place although as you mention some may not be matched up. The other thing that needs to happen is that the components need to heed the threading information coming from the driver instead of looking for it in the environment. To be clear, this is the way that CESM would like for it to work and we are aware that it doesn't work that way now.

@worleyph
Copy link
Contributor

worleyph commented Mar 1, 2016

To be clear, this is the way that CESM would like for it to work and we are aware that it doesn't work that way now.

Yeah - but it looks like it should be working that way now :-). The code is

   subroutine seq_comm_setnthreads(nthreads)
 #ifdef _OPENMP
     if (nthreads < 1) then
        call shr_sys_abort(subname//' ERROR: nthreads less than one')
     endif
     call omp_set_num_threads(nthreads)
 #endif

   end subroutine seq_comm_setnthreads

Seems like omp_set_num_threads should be controlling the number of threads visible to the components. If I get a chance I'll try poking at this as well.

@jedwards4b
Copy link
Contributor

I believe that you need to set the drv_threading namelist variable.
And regardless of what the driver does it doesn't make any difference if
the components aren't listening.

On Mon, Feb 29, 2016 at 7:26 PM, worleyph [email protected] wrote:

To be clear, this is the way that CESM would like for it to work and we
are aware that it doesn't work that way now.

Yeah - but it looks like it should be working that way now :-). The code
is

subroutine seq_comm_setnthreads(nthreads)
#ifdef _OPENMP
if (nthreads < 1) then
call shr_sys_abort(subname//' ERROR: nthreads less than one')
endif
call omp_set_num_threads(nthreads)
#endif

end subroutine seq_comm_setnthreads

Seems like omp_set_num_threads should be controlling the number of threads
visible to the components. If I get a chance I'll try poking at this as
well.


Reply to this email directly or view it on GitHub
#67 (comment).

Jim Edwards

CESM Software Engineer
National Center for Atmospheric Research
Boulder, CO

@worleyph
Copy link
Contributor

worleyph commented Mar 1, 2016

I would have guessed that the component listening or not would not matter if the driver successfully defined the number of available threads using omp_set_num_threads. I think that this is more than just an advisory - it sets a hard upper limit?

A web search is confusing however. One statement ...

 Overrides the setting of the OMP_NUM_THREADS environment variable, and specifies the
 number of threads to use for a subsequent parallel region by setting the first value of num_list 
 for OMP_NUM_THREADS.

seems to indicate that this is how it is supposed to work.

However, the current default in ACME? in CIME? is

drv_threading = .false.

so we are not even trying to use this. First step is to change this and see what happens.

@amametjanov , do you want to take a shot at this? I think that you have an experiment set up to try this on Mira or Cetus? (Perhaps @amametjanov is not a member of this repository. I'll make thin comment on the associated ACME github issue.)

@jedwards4b
Copy link
Contributor

We need more detail in this ticket - some of the components ignore the thread information that the driver sends them.

@douglasjacobsen
Copy link
Member Author

@jedwards4b Is there something specific you would like added here? I think this ticket is trying to document how a component should be handling thread information from the driver. But I'd argue that if a component is ignoring that, it's handling it incorrectly and that should be reported as a bug.

jayeshkrishna pushed a commit that referenced this issue Aug 16, 2016
changes to pioc.c, pio_lists.c to support async, also some temporary code
@jgfouca
Copy link
Contributor

jgfouca commented May 10, 2017

@worleyph @rljacob , we're trying to figure out what to do with this ticket, thoughts?

@jgfouca jgfouca added ready and removed in progress labels May 10, 2017
@amametjanov
Copy link
Member

ACME sets drv_threading=.true. in drv_in namelist and components are picking up the correct number of threads. I think this can be closed.

@rljacob
Copy link
Member

rljacob commented May 10, 2017

Or change the title to make it a CESM issue.

@rljacob
Copy link
Member

rljacob commented May 10, 2017

Actually close it since Doug brought up wrt ACME.

@rljacob rljacob closed this as completed May 10, 2017
@ghost ghost removed the ready label May 10, 2017
jgfouca pushed a commit that referenced this issue Feb 20, 2019
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

7 participants