CH-Earth / summa

Structure for Unifying Multiple Modeling Alternatives:
http://www.ral.ucar.edu/projects/summa
GNU General Public License v3.0
79 stars 103 forks source link

updated whats-new file for v3.2.0 release #531

Closed wknoben closed 1 year ago

wknoben commented 1 year ago

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

andywood commented 1 year ago

Hi Wouter, thanks for digging up the conversation -- it's good that we were able to land on including it based on that analysis.

On Wed, Jul 5, 2023 at 9:16 PM Wouter Knoben @.***> wrote:

@.**** commented on this pull request.

On docs/whats-new.md https://github.com/CH-Earth/summa/pull/531#discussion_r1253875637:

We looked at this before when it was implemented in #465 https://github.com/CH-Earth/summa/pull/465. Copying the relevant parts:

According to this https://stackoverflow.com/questions/22122494/is-there-a-cpu-time-performance-penalty-in-fortran, 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.

Great analysis, Wouter -- thanks for digging into this. Given that we mostly run our models in big split-domain (or some kind of parallelization) modes, I don't think it's a big enough hit to object to adding it, and without the if() statement. For instance, for a 20-node run of a CONUS domain it might end up adding 20-30s, maybe 2 min at much higher resolution. Happy with what others think.

— Reply to this email directly, view it on GitHub https://github.com/CH-Earth/summa/pull/531#discussion_r1253875637, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKARJJ2C7AJDAMQNOUZFTXOYUZZANCNFSM6AAAAAAZ7SHDL4 . You are receiving this because you commented.Message ID: @.***>

wknoben commented 1 year ago

No problem :)