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

compute cpu time for each HRU and time step #465

Merged
merged 2 commits into from
Oct 12, 2021

Conversation

martynpclark
Copy link
Collaborator

@martynpclark martynpclark commented Jun 14, 2021

Make sure all the relevant boxes are checked (and only check the box if you actually completed the step):

  • closes #xxx (identify the issue associated with this PR)
  • tests passed
  • new tests added
  • science test figures
  • checked that the new code conforms to the SUMMA coding conventions
  • ReleaseNotes entry

@martynpclark
Copy link
Collaborator Author

This computes the CPU time spend on each HRU and time step. While the elapsed wall clock time is desired by users, the precision may be as low as 0.01 seconds. In many cases, it will be preferable to look at metrics like the number of flux calls. Nevertheless, the elapsed wall clock time is provided to meet user requests.

@arbennett
Copy link
Member

@martynpclark this looks good and I think is a nice feature to add, but I have one request - could you add a quick entry to the docs/whats-new.md list under a new header Develop?

@martynpclark
Copy link
Collaborator Author

Updated whats-new file

@andywood
Copy link
Collaborator

andywood commented Oct 9, 2021

Can this be included as a user option? ie lumped in with 'verbose' option? For a 50 yr 3-hourly run of a CONUS model at intermediate scale (~100k units) this leads to a new set of 13.5 billion time calculations included. I don't know the cost of this, but I'd guess that for most applications this level of info is not of interest, and could be shut off. That said it might lead to 13.5 if statements which also have a cost. If the cost is trivial, ie <0.5% of the overall run time, I'd say go ahead. I have no idea.

@wknoben
Copy link
Collaborator

wknoben commented Oct 12, 2021

According to this, execution time for cpu_time() is not negligible for very large numbers of computations.

Some quick (online Fortran compiler) testing shows that:

program cputimer
  implicit none
  integer :: i
  integer, parameter :: n = 1000000
  real(8) :: t1, t2, t
  call cpu_time(t1)
  do i = 1, n
    call cpu_time(t)
  end do
  call cpu_time(t2)
  write(*,*) (t2-t1)/n
end program

has an average execution time per iteration of 1.099E-6 s. If this translates directly from this online tool to regular compiled Fortran code, we're looking at an extra 50 * 365.25 * 8 * 100000 * 1.099E-6 ~= 4.5 hr of total time added to these CONUS runs. On a per-HRU basis this is not excessive though, at approx. 0.16 s per HRU

I also looked at the time cost of an IF-statement like so:

program iftimer
  implicit none
  integer :: i
  integer, parameter :: n = 1000000
  logical, parameter :: calcTime = .FALSE.
  real(8) :: t1, t2, t
  call cpu_time(t1)
  do i = 1, n
    IF (calcTime) THEN
      call cpu_time(t)
    END IF
  end do
  call cpu_time(t2)
  write(*,*) (t2-t1)/n
end program

This gives an average execution time per loop iteration of 2.032E-9 s, meaning approximately 30 s added to the CONUS run (approx. 0.0003 s per HRU).

On a per-HRU basis either approach is pretty much negligible but for very large domains these calls may start to add up. As far as I can see there are two conflicting principles:

  • On the one hand, only executing computations when required makes sense;
  • On the other hand, the code may be kept simpler if we don't excessively use if statements and flags.

I'm not sure what to suggest but figured I'd share the analysis at any rate to put some (estimated) numbers to this problem.

@andywood
Copy link
Collaborator

andywood commented Oct 12, 2021 via email

@martynpclark martynpclark merged commit 9195fda into CH-Earth:develop Oct 12, 2021
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 this pull request may close these issues.

4 participants