New Basic Operations#454
Conversation
…d variants. also added 2d copy all operations follow the pattern a + b = c, rather than modifying input fields
romanc
left a comment
romanc
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for adding those.
We could talk about consistency, e.g.
add_2dvsset_value_2Dadjustmentfactor_stencilvsadjust_divide_stencil- imo we could even loose the_stencilsuffixset_IJ_mask_valuealso looks out of sync
However, those were all previously there and would need - to be nice with our users - properly deprecated for a release before being removed.
|
|
||
|
|
||
| def adjustmentfactor_stencil(adjustment: FloatFieldIJ, q_out: FloatField) -> None: | ||
| def copy_2d(input: FloatFieldIJ, output: FloatFieldIJ) -> None: |
There was a problem hiding this comment.
what is the use-case for this?
If you call this stencil with two 2D-Fields you can call it on interval(...) so it matches the 3d stencil if that is done forward as well.
There was a problem hiding this comment.
I'm not sure I follow you here. Are you just saying that I should swap interval(0, 1) to interval(...)? That's fair. it's done.
I don't have a use for the stencil at the moment, but I want to make sure I include 2d variants of everything so people have a full set available.
There was a problem hiding this comment.
reverted interval(...) back to interval(0, 1) to avoid over computing
There was a problem hiding this comment.
Do we only need this stencil specifically because input and output are typehinted as 2D?
There was a problem hiding this comment.
If that's the case why not extend to IntField and IntField2d`?
|
|
||
|
|
||
| def set_value(q_out: FloatField, value: Float) -> None: | ||
| def add_2d(input_1: FloatFieldIJ, input_2: FloatFieldIJ, output: FloatFieldIJ) -> None: |
|
Back to draft for the weekend - I am adding more stencils to the batch |
oelbert
left a comment
oelbert
left a comment
There was a problem hiding this comment.
This adds a lot of stencils and I guess I'm curious how much we need to specify each of them here, especially 2D and 3D versions? How often are we only adding or only dividing in our numerics?
|
|
||
|
|
||
| def adjustmentfactor_stencil(adjustment: FloatFieldIJ, q_out: FloatField) -> None: | ||
| def copy_2d(input: FloatFieldIJ, output: FloatFieldIJ) -> None: |
There was a problem hiding this comment.
Do we only need this stencil specifically because input and output are typehinted as 2D?
|
|
||
|
|
||
| def adjustmentfactor_stencil(adjustment: FloatFieldIJ, q_out: FloatField) -> None: | ||
| def copy_2d(input: FloatFieldIJ, output: FloatFieldIJ) -> None: |
There was a problem hiding this comment.
If that's the case why not extend to IntField and IntField2d`?
Surprisingly often. I found that, when I don't have these stencils, I just end up wrapping the simple one-liner adds into other stencils, even if it doesn't make a lot of sense scientifically for them to be there, but once I had these stencils I started was more free to write discrete scientific blocks full of smaller, more narrowly tuned stencils, which has the added effect of allowing stencils names to more accurately act as (in my opinion) the best documentation. |
|
To you other point - yes and no. 3d vs 2d need to be their own entity, because 3d can use computation/interval It would be nice to say "this stencil can use any type/shape so long as all fields are of the same type/shape" and have one stencil work everything, but I think we are quite a way from having something like that. |
|
A good example of their use can be found here: Fortran v NDSL (WIP) |
4 participants
Added 2D and 3D addition, subtraction, multiplication, division and a 2D copy (3D already exists) to
basic_operations.py.There are two sets of the mathematics stencils, one which modify inputs (
a = a + b) and one which do not (a + b = c).New stencils are covered by tests in
test_basic_operations.py.