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

Feature/cesm timing #744

Merged
merged 4 commits into from
Sep 5, 2017
Merged

Conversation

dgergel
Copy link
Contributor

@dgergel dgergel commented Sep 1, 2017

  • closes #xxx
  • tests passed
  • new tests added
  • science test figures
  • ran uncrustify prior to final commit
  • ReleaseNotes entry

See #743 for discussion of what this PR addresses (in part).

@@ -160,8 +168,10 @@ vic_cesm_run(vic_clock *vclock)
// if save:
if (vclock->state_flag) {
// write state file
debug("writing state file for timestep %zu", current);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget how this works. Are debug() statements always written out or only when we specify a certain logging level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not always. Only when we specify a certain logging level, and I believe compiling with the debug flag for an RI case does the same thing, even though we can't do that for an RBR case.

@@ -131,9 +131,6 @@ main(int argc,
// start vic run timer
timer_start(&(global_timers[TIMER_VIC_RUN]));

timer_init(&(global_timers[TIMER_VIC_FORCE]));
timer_init(&(global_timers[TIMER_VIC_WRITE]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we not have to initialize timers? If that is done as part of timer_start(), then we can also remove timer_init() from:

  • vic/drivers/shared_all/include/vic_driver_shared_all.h
  • vic/drivers/shared_all/src/timing.c
  • tests/unit/shared/test_timing.py (vic_lib.timer_init(timer))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually timer_init() is called from timer_start(), so you do not have to call it separately, but you cannot delete it from the other files as I suggest above, so this should be OK.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could combine timer_init() and timer_start(), but just leave as is for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, the reason we have separate timer_init and timer_start functions is for the CESM driver case where the vic_cesm_run function needs to "continue" a previously initialized timer. I don't think the image driver has a use for the separate functions. And yes, timer_start = timer_init + timer_continue.

@jhamman
Copy link
Member

jhamman commented Sep 1, 2017 via email

@bartnijssen
Copy link
Member

What would be the use case of running timer_init() without ever calling a timer_start()? Keep in mind that every timer_start() already includes a call to timer_init()?

@dgergel
Copy link
Contributor Author

dgergel commented Sep 1, 2017

@jhamman @bartnijssen the timing tables look good now (for what we're changing in this PR). I'll fix the secs/day columns in a separate PR. For reference:

------------------------------ VIC TIMING PROFILE ------------------------------

  Date                      : Fri Sep  1 18:49:29 2017
  Compiler                  : icc (Intel(R) C++ gcc 4.3 mode)
  Machine                   : r24i4n1
  VIC User                  : gergel
  VIC Version               : unset
  VIC GIT Version           : 5.0.1 February 1, 2017
  VIC_DRIVER                : CESM

  Global Param File         : vic.globalconfig.txt
  Domain File               : /p/work2/projects/rasm/inputdata/share/domains/domain.lnd.wr50a_ar9v4.100920.nc
  Start Date                : 1948-09-01-00000
  Stop Date                 : 0000-00-00
  Nrecs                     : 1
  Model Timestep (seconds)  : 3600
  Snow Timestep (seconds)   : 3600
  Runoff Timestep (seconds) : 3600
  Atmos Timestep (seconds)  : 3600

  MPI Processes             : 64
  OPENMP Threads            : 1
  Total pes active          : 64
  pes per node              : 72

  Overall Metrics
  ---------------
    Model Cost       : 288149 pe-hrs/simulated_year
    Model Throughput : 0.00533057 simulated_years/day

  Timing Table:
|------------|----------------------|----------------------|----------------------|----------------------|
| Timer      | Wall Time (secs)     | CPU Time (secs)      | Wall Time (secs/day) | CPU Time (secs/day)  |
|------------|----------------------|----------------------|----------------------|----------------------|
| Init Time  |              1.68781 |                 0.52 |              40.5075 |                12.48 |
| Run Time   |              1848.57 |              1846.94 |              44365.8 |              44326.6 |
| Final Time |            0.0124612 |                 0.01 |             0.299068 |                 0.24 |
| Total Time |              1850.27 |              1847.47 |              44406.6 |              44339.3 |
|------------|----------------------|----------------------|----------------------|----------------------|
| Force Time |             0.148592 |                 0.18 |               3.5662 |                 4.32 |
| Write Time |              470.665 |               470.31 |              11295.9 |              11287.4 |
|------------|----------------------|----------------------|----------------------|----------------------|


------------------------------ END VIC TIMING PROFILE ------------------------------

@bartnijssen bartnijssen merged commit b63a0d9 into UW-Hydro:develop Sep 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants