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

recursive timer calls to CPL:RUN #1981

Closed
worleyph opened this issue Dec 12, 2017 · 6 comments · Fixed by #2266
Closed

recursive timer calls to CPL:RUN #1981

worleyph opened this issue Dec 12, 2017 · 6 comments · Fixed by #2266

Comments

@worleyph
Copy link
Contributor

In cime/src/drivers/mct/main/cime_comp_mod.F90 is logic of the form:

     call t_drvstartf ('CPL:ATMOCN1',cplrun=.true.,barrier=mpicom_CPLID,hashint=hashint(4))
  ...
     if (do_budgets) then
  ...
        call t_drvstartf ('CPL:BUDGET0',cplrun=.true.,budget=.true.,barrier=mpicom_CPLID)
  ...
        call t_drvstopf ('CPL:BUDGET0',cplrun=.true.,budget=.true.)
     endif
  ...
     call t_drvstopf  ('CPL:ATMOCN1',cplrun=.true.,hashint=hashint(4))

and also later in the code for the timers 'CPL:ATMOCNP' and 'CPL:ATMOCN2 (all for 'CPL:BUDGET0').

With cplrun=.true., 'CPL:RUN' t_startf is called twice before 'CPL:RUN' t_stopf (twice), resulting in a "recursive" call. This does not seem to do any harm - GPTL can handle this - but I doubt that this was intended. I would like the developers who added the do_budget code to revisit this and determine whether this was intentional. (The fix is to remove 'cplrun=.true.' from the calls to

 call t_drvstartf ('CPL:BUDGET0',cplrun=.true.,budget=.true.,barrier=mpicom_CPLID)
 call t_drvstopf ('CPL:BUDGET0',cplrun=.true.,budget=.true.)

in these three situations.

Not sure who can respond to this, but since it is in CIME, am assigning @jgfouca initially (sorry Jim).

@jgfouca
Copy link
Member

jgfouca commented Dec 12, 2017

Going to pass this hot potato to @rljacob

@jgfouca jgfouca assigned rljacob and unassigned jgfouca Dec 12, 2017
@worleyph
Copy link
Contributor Author

worleyph commented Apr 4, 2018

@rljacob , did you ever hear back from the do_budget developers? Should I submit a PR to make the change that (I think) is needed?

@rljacob
Copy link
Member

rljacob commented Apr 4, 2018

No I did not. Is that logic still there?

@worleyph
Copy link
Contributor Author

worleyph commented Apr 4, 2018

Yes.

@amametjanov
Copy link
Member

Does the recursive call cause a memory (runaway) leak? The run_e3sm script for high-res runs turns this on:

xmlchange --id BUDGETS --val TRUE

whereas runs with create_test/create_newcase do not.

@worleyph
Copy link
Contributor Author

worleyph commented Apr 7, 2018

No - it is innocuous. GPTL can handle "recursive" calls (just nested calls of timers with the same name), but it silly to have this as a recursive call and it makes the timing output a little misleading.

rljacob added a commit that referenced this issue Apr 10, 2018
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

BFB
rljacob added a commit that referenced this issue Apr 13, 2018
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

[BFB]
jgfouca pushed a commit that referenced this issue Apr 27, 2018
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

[BFB]
rljacob pushed a commit that referenced this issue Apr 12, 2021
Resolve compset issue

A subtle bug fix - the discovery of compsets needs to have information about available components so that it can look in the correct cime_config directories for config_compsets.xml files.

Test suite: scripts_regression_tests.py + cesm F cases
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:
rljacob added a commit that referenced this issue Apr 12, 2021
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

[BFB]
rljacob added a commit that referenced this issue Apr 21, 2021
The checkpoint timing files, in timing/checkpoints, are named

cesm_timing_<simulation_date_and_time>.

Replace the cesm_timing prefix with model_timing prefix.

Also eliminate the (unnecessary) recursive timer call to CPL:RUN
when do_budgets is .TRUE.

Fixes #2264
Fixes #1981

[BFB]
wlin7 pushed a commit that referenced this issue Feb 26, 2023
…-ne30L128-ic

Automatically Merged using E3SM Pull Request AutoTester
PR Title: Switch to new ne30L128 IC with hydrometeors clipped
PR Author: brhillman
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants