Add LevelPlaceholderSpace #1987
Conversation
dennisYatunin
force-pushed
the
dy/level_placeholder
branch
3 times, most recently
from
2e4237c to
cff25c6
Compare
dennisYatunin
force-pushed
the
dy/level_placeholder
branch
from
cff25c6 to
83f7959
Compare
| placeholder_space( | ||
| current_space::Spaces.AbstractPointSpace, | ||
| parent_space::Spaces.AbstractFiniteDifferenceSpace, | ||
| ) = LevelPlaceholderSpace() | ||
| placeholder_space( | ||
| current_space::Spaces.AbstractSpectralElementSpace, | ||
| parent_space::Spaces.ExtrudedFiniteDifferenceSpace, | ||
| ) = LevelPlaceholderSpace() |
There was a problem hiding this comment.
Why should these return level spaces?
There was a problem hiding this comment.
Suppose that, as in the case of the nonorographic gravity wave parametrization, we have a broadcast over extruded/vertical fields and horizontal/point fields that needs to be evaluated on the GPU. The parent_space in this case is the space of the extruded/vertical output field, and the current_space is either the same as the parent_space or just one level of the parent_space. In the first case we get a PlaceholderSpace, and in the second case we get a LevelPlaceholderSpace.
There was a problem hiding this comment.
That may makes sense for the second method, but the first method looks to me like it should return the parent space.
There was a problem hiding this comment.
That’s after all how we support point-sphere broadcasting.
There was a problem hiding this comment.
If the first method returned parent_space instead of LevelPlaceholderSpace, there would be multiple non-placeholder spaces passed to the GPU. My impression is that we want only one non-placeholder space (the output of the broadcast) passed to the GPU, and all other spaces can be reconstructed from that non-placeholder space when they are needed.
With the current design, if a PointField is used in a broadcast expression with a FiniteDifferenceField as the output, a LevelPlaceholderSpace will be returned by placeholder_space and a PointSpace will be returned by reconstruct_placeholder_space. If the first method returned parent_space instead of LevelPlaceholderSpace, reconstruct_placeholder_space would return a FiniteDifferenceSpace for a PointField, which makes no sense to me.
There was a problem hiding this comment.
Yeah, sorry, I meant ..., but the first method looks to me like it should a type similar to the parent space..
if a PointField is used in a broadcast expression with a FiniteDifferenceField as the output
Given this condition, I agree with you, but this condition is not enforced in placeholder_space, which is why this change looks a bit odd to me.
|
@dennisYatunin, please make sure to add tests so that this new code is exercised. |
This PR adds a
LevelPlaceholderSpace, which is needed to pass mixtures of extruded 3D and level 2D fields onto the GPU. Specifically, this will enable us to perform the column accumulation operation needed for the non-orographic gravity wave parametrization in ClimaAtmos (CliMA/ClimaAtmos.jl#3267).