-
Notifications
You must be signed in to change notification settings - Fork 191
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
Forcing API refactor #444
Forcing API refactor #444
Conversation
@suyashbire1 we can use this pattern to implement a Happy to work on that once this is merged. |
…ate the use of SimpleForcing
Codecov Report
@@ Coverage Diff @@
## master #444 +/- ##
=========================================
- Coverage 71.85% 71.55% -0.3%
=========================================
Files 25 26 +1
Lines 1400 1417 +17
=========================================
+ Hits 1006 1014 +8
- Misses 394 403 +9
Continue to review full report at Codecov.
|
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.
Looks good! I might just add a couple of examples to the docstrings.
Should probably also add a SimpleForcing
benchmark to benchmark_forcing_functions.jl
but that doesn't have to be in this PR.
There are now two ways to specify a forcing function. If you want to use parameters then you must use the more low-level version and pass a function straight to ModelForcing
.
This choice is hard to document through docstrings, which just means we have to document it clearly in the documentation page on "model setup". I've been meaning to write that page for a while, hopefully next week...
We can extend u_forcing = SimpleForcing((x, y, z, t, parameters) -> something, parameters=user_defined_parameters) What do you think? Interestingly, putting parameters in the forcing object obviates the need for model.parameters. Ultimately this design may be preferred; I suppose we will see. |
Hmmm, yeah I guess |
To make bc = SimpleBoundaryCondition(Flux, (x, y, t, parameters) -> something, parameters=parameters) Not suggesting we do it now, but something to think about for the future. |
…ments Forcing API refactor Former-commit-id: 8876640
This PR changes the name of the
Forcing
constructor toModelForcing
to clarify the difference between constructors for forcing objects on single fields, and constructors for theNamedTuple
of forcing objects for all model fields. It also conventionalizes the name of the fields inModelForcing
, so thatis used to apply the forcing function
u_forcing
tou
.In addition it adds a new type "
SimpleForcing
", which permits users to pass functions ofx
,y
,z
, andt
. A few validating lines in theModelForcing
constructor ensure that forcing functions foru
,v
, andw
are applied at the correct locations.For example, we can now write
To create a forcing on temperature that decays exponentially in z and oscillates in time.
To test this functionality, this PR introduces an example "
ocean_convection_with_plankton
" that usesSimpleForcing
to model the growth and decay of a plankton-like tracer.