-
Notifications
You must be signed in to change notification settings - Fork 0
Add simple C14 evaluation cumulative timer #40
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ module advance_xp2_xpyp_module | |
| torch_model_load, & | ||
| torch_model_forward, & | ||
| torch_delete | ||
| use code_timer_module, only: & | ||
| timer_t, timer_start, timer_stop | ||
|
|
||
| implicit none | ||
|
|
||
|
|
@@ -56,6 +58,9 @@ module advance_xp2_xpyp_module | |
| ndiags5 = 5 | ||
|
|
||
| type(torch_model) :: C14_neural_net | ||
| ! Global thread-unsafe timer for measuring the cumulative execution time of C14 | ||
| ! evaluation | ||
| type(timer_t) :: C14_timer_total | ||
|
|
||
| contains | ||
|
|
||
|
|
@@ -79,6 +84,9 @@ subroutine setup_C14_ML_xp2_xpyp( c14_net_filepath ) | |
|
|
||
| call torch_model_load(C14_neural_net, c14_net_filepath, torch_kCPU) | ||
|
|
||
| ! Initialise timer | ||
| C14_timer_total = timer_t() | ||
|
|
||
| end subroutine setup_C14_ML_xp2_xpyp | ||
|
|
||
| subroutine clean_up_C14_ML_xp2_xpyp() | ||
|
|
@@ -98,6 +106,9 @@ subroutine clean_up_C14_ML_xp2_xpyp() | |
|
|
||
| call torch_delete( C14_neural_net ) | ||
|
|
||
| ! Print the time | ||
| write(unit=fstdout, fmt='(a,g,a)') "C14 total evaluation time: ", C14_timer_total % time_elapsed, " [s]" | ||
|
|
||
| end subroutine clean_up_C14_ML_xp2_xpyp | ||
|
|
||
| !============================================================================= | ||
|
|
@@ -607,7 +618,7 @@ subroutine advance_xp2_xpyp( nzm, nzt, ngrdcol, sclr_dim, sclr_tol, gr, sclr_idx | |
| ! Once this is working we can move towards batching a column at a time or multiple | ||
| ! columns at once. | ||
| if ( l_c14_ml ) then | ||
|
|
||
| call timer_start(C14_timer_total) | ||
| ! Interpolate Lscales from thermal to momentum grid | ||
| Lscale_up_zm(:,:) = zt2zm_api( nzm, nzt, ngrdcol, gr, Lscale_up(:,:), zero_threshold ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we pretend the interpolation from tracer to momentum grid is part of the net. This probably makes sense since this step is not needed in noML-mode. But it made me wonder why the net can't do that itself? Meaning
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adconnolly I believe you input on that may be necessary ;-) [Myself I lack the knowledge and context]
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could add 2 Lscales, one above and one below, for all but zm(1) point where the surface is. Because c14 doesn't matter at the surface, this could be interesting to try, at least offline to see if it provides any additional skill. I'm skeptical it will because the gradients dL/dz never appear in the existing physical closures model, and the drawback would be having a bigger net. If it really is just the average that matters, I think it would be a waste of computational expense to have a larger network. Another point, the interpolation of the Lscale does happen in the non-ML model but it is just the "master" Lscale that gets interpolated to combine with sqrt(TKE) to get a single time scale at zm points. I've check this before and I'm pretty sure it is the master length scale that gets interpolated, but it could just as well be the underlying _up and _down Lscales or the resultant inverse time scale that gets interpolated. If it is the _up and _down Lscales that get interpolated I suppose we could pass those through and save some re-interpolation, but we'd need to look at where time scale, tau, or its inverse gets calculated |
||
| Lscale_down_zm(:,:) = zt2zm_api( nzm, nzt, ngrdcol, gr, Lscale_down(:,:), zero_threshold ) | ||
|
|
@@ -633,7 +644,7 @@ subroutine advance_xp2_xpyp( nzm, nzt, ngrdcol, sclr_dim, sclr_tol, gr, sclr_idx | |
| C14_1d(i,k) = one_third * c14_ml_output(1) | ||
| end do | ||
| end do | ||
|
|
||
| call timer_stop(C14_timer_total) | ||
| endif ! l_c14_ml | ||
|
|
||
| ! Are we solving for passive scalars as well? | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To discuss: I couldn’t really decide on the format for measurement. If I were to follow the CLUBB-TIMER convention if should be
f10.4. I went with general since I though we don't care really and the fortran library can decide for what is the most appropriate format for the given value.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on this. The only issue I could see with
f10.4is if we test models that take only milliseconds or hours to evaluate, both of which I doubt will happen (?). Anyway I think going with general won't do harm!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair given the timer overheads I don't think our measurement is precise enough to tell anything significant for
<0.1ms, so accuracy wise 4 decimal places should be OK. But if we are happy not to follow the CLUBB previous output format I am happy :-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick update regarding this @Mikolaj-A-Kowalski. When I compile this with gfortran it complained about Fortran 2018 standards:
Error: Fortran 2018: positive width required at (1)withfmt='(a,g,a)'.Apparently a more general
fmt='(a,g0,a)'is needed? At least it compiled without problem.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad! Thank you for catching that! Do you wish to submit a quick PR with a fix or you prefer I do it?