YAML specification of diffusion-reaction models#267
YAML specification of diffusion-reaction models#267LawrenceDior wants to merge 9 commits intothetisproject:mainfrom
Conversation
| reserved_symbols = { | ||
| "pi", "e", | ||
| "x", "y", "z", "t", | ||
| "u", "u_x", "u_y", "u_z"} |
There was a problem hiding this comment.
I guess a discussion point is whether we want all of these (and any more) as reserved symbols. I can imagine pi, e, x, y, t being useful for if you have a source term that depends on them. Probably not z in a 2D model.
There was a problem hiding this comment.
Agreed -- see separate comment. For z, I suppose the question is whether or not this class might also be used in tandem with 3D solvers.
| pytz | ||
| networkx | ||
| pyyaml | ||
| sympy |
There was a problem hiding this comment.
At the minute the implementation converts yml->dict->sympy->UFL. Maybe it is possible to do yml->dict->UFL? If so, it's probably best to leave for future work.
| reserved_symbols = { | ||
| "pi", "e", | ||
| "x", "y", "z", "t", | ||
| "u", "u_x", "u_y", "u_z"} |
There was a problem hiding this comment.
These symbols are reserved to allow reaction terms to contain pi, e, spatial coordinates, time, the flow field, and individual components of the flow field.
I'm not aware of any issues with pi or e, because these are just constants. However, at present, parse_tracers_from_dict will not work properly if it encounters a reaction term containing any of these other symbols. It expects all symbols to correspond to either constants or species.
The simplest way to solve this problem would be to remove everything except pi and e from reserved_symbols, but it might be worth considering alternatives if people would like to be able to include spatial coordinates, etc. in reaction terms.
|
Note: this could also be used for Thetis' sediment model. |
joewallwork
left a comment
joewallwork
left a comment
There was a problem hiding this comment.
Thanks, Lawrence! This is great. A few comments and requests. e.g. it'd be nice if you could demonstrate the networkx usage in the Gray-Scott example.
Sorry it took a while to get round to reviewing.
| self.species[k] = { | ||
| "name": v["name"], | ||
| "index": i, | ||
| "diffusion": float(diffusion_term), |
There was a problem hiding this comment.
So at the minute we are only supporting constant diffusion terms, which is fine. To support spatially varying diffusivity and coefficients, I guess we would need the functionality to load from file or something. Probably best to just leave for now until someone needs to be able to do it.
- Added networkx to requirements.txt (required for ADR_Model.dependency_graph) - Adjusted the way load_tracers_2d sets the name and fname parameters of add_tracer_2d - The preserve_order argument of read_tracer_from_yml is now True by default - Corrected read_tracer_from_yml docstring
joewallwork
force-pushed
the
yml-lfd120
branch
from
454d657 to
b25919d
Compare
|
With @LawrenceDior's permission, I made the requested changes. |
2 participants
read_tracer_from_ymlreads tracer diffusivities and reaction terms from a .yml file and returns anOrderedDictof Firedrake and UFL objects representing this information.The
ADR_Modelclass is responsible for model validation and the parsing of reaction terms.The
load_tracers_2dmethod of theFlowSolver2dclass callsread_tracer_from_ymland adds the corresponding tracers to theFlowSolver2dobject.examples/reaction/gray_scott_io.pycontains a working example of a diffusion-reaction model loaded from disk.See
reaction_models/gray-scott.ymlfor an example of a .yml file in the expected format.