Closed axch closed 7 years ago
This looks good and runs nicely on my machine, with the caveat that I can reproduce the error described in #73.
A few thoughts:
I'm a little surprised to the see that the model_run
subroutine is consuming about 40% of the runtime in reduced gravity mode. There's very little happening in there that isn't farmed out to subroutines. Is the array shuffling causing this? It's not a big issue - we should definitely focus our performance improvements on the matrix solve - but is a little surprising.
I'm not a fan of the subroutine name enforce_moderate_free_surface
. The subroutine forces the layer thicknesses, free surface, and depth field to be consistent with each other, not moderate. How do you feel about one of us changing it to enforce_depth_thickness_consistency
?
I feel similarly about the isopycnal_correction
subroutine. A better name would be barotropic_correction
.
Responses:
model_run
is concentrated in the actual time-stepping. In particular, moving the array allocation between the heap and the stack did not cause a performance effect that I could detect.enforce_depth_thickness_consistency
is fine with me. I was guessing when I chose the other name. Care to do the honors?barotropic_correction
likewise. I think I was keying off the comments nearby, which are worth making more precise as well. Care to do the honors there too?LGTM. Merge at will :)
Split the core program up into subroutines. Fixes #70.