Calculate tendencies on boundaries #677
Conversation
…ency calculations
Codecov Report
@@ Coverage Diff @@
## master #677 +/- ##
==========================================
- Coverage 77.99% 77.98% -0.01%
==========================================
Files 120 121 +1
Lines 2413 2444 +31
==========================================
+ Hits 1882 1906 +24
- Misses 531 538 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks great! Certainly would be easy to make changes to the momentum and tracer equations now.
I should add a page for private API documentation (so all these docstrings make it into the documentation, actually that's just #599).
| `parameters` is a `NamedTuple` of scalar parameters for user-defined forcing functions | ||
| and `time` is the physical time of the model. |
There was a problem hiding this comment.
I thought model.parameters could be anything passed in by the user, although we usually use named tuples (so do most people for these kinds of parameters).
There was a problem hiding this comment.
That's true about model.parameters --- except that kernels will fail to compile on the GPU unless it only contains simple objects.
There was a problem hiding this comment.
Hmm, ok, I will change the docstring to be more accurate.
There was a problem hiding this comment.
I think we're planning to get rid of model.parameters but whatever we replace it (e.g. model.forcing.parameters) with should probably work the same way for writing forcing functions so thankfully the docstrings will still be correct then.
There was a problem hiding this comment.
Should we merge this and address when we address #682 ?
|
Should probably release v0.25.2 once this is merged as this PR fixes a bug. |
|
@ali-ramadhan I propose we merge now and edit the tendency function docstrings when we address #682, since we'll have to do it then anyways. |
|
Sounds good to me! Will release v0.25.2. |
This PR modifies the time-stepping algorithm so that the tendencies for wall-normal velocity components are calculated on boundaries in
Boundeddirections.In other words, when
xisBounded, the algorithm now calculatesGuon east boundaries, wherei=Nx+1. PreviouslyGuwas only calculated on west boundaries wherei=1. The same applies toyandz.This change is necessary because, in general, the values of
Gu,Gv, andGwonx,y, orzboundaries are needed to impose predictor velocity and pressure boundary conditions whenx,y, orzareBounded--- respectively.The changes in this PR also motivated a slight refactoring of the way "equations" are specified. There is now a file dedicated to equation / tendency specification, called
velocity_and_tracer_tendencies.jlinsrc/TimeStepping.Resolves #259 (since it implements a simple / minimal abstraction for equations).