-
Notifications
You must be signed in to change notification settings - Fork 17
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
New wind variables at surface #75
Conversation
Tagging @shlyaeva |
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.
LGTM
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.
I am probably making a mountain out of a molehill, but what is the "surface" that these standard names are referring to? For example my understanding is that when meteorologists report surface winds they are usually reporting winds at ten meters above the surface, which we already have a suffix for (at_10m
).
Alternatively my (extremely limited) understanding of boundary layer turbulence assumes that the wind speeds at the actual surface are exactly zero relative to the surface itself, so any local non-zero wind speed must be defined at some height above the actual surface.
Given this, if this is a quantity defined on model levels, would something like at_surface_adjacent_layer
or at_bottom_interface
be a better suffix?
Anyways, I certainly won't hold up this PR for this as it sounds like it is needed soon, but I am just worried that we are potentially adding some ambiguity here. Of course I'm happy to hear if people disagree. Thanks!
I agree with this comment but I do not have anything useful to add as I'm not sure what |
The names in this PR were previously part of #71 (but were removed from that PR for other reasons), and this same discussion came up there. See comments from @BenjaminTJohnson and @mikecooke77 on that PR. Also tagging @shlyaeva |
@svahl991 Thanks for pointing me to the extra discussion! I think that all makes sense to me. Just to make sure it is as clear as possible, could we possibly update the long names to make it obvious that these wind fields are whatever wind variable is closest to the surface? So using
|
I've now updated the long names in this PR with this clarification, as requested. |
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.
Thanks for humoring me! Everything looks good on my end now.
Thanks @svahl991. Let's wait for @mkavulich to review and merge? |
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.
Thanks for adding
Both this PR and #76 have several approvals and don't seem to have any ongoing discussions. Is it possible to get them merged soon? |
We are unfortunately still waiting for approval from the NOAA side. I pinged @mkavulich and @dustinswales in the other PR #76 a few days ago. |
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.
@climbfuji Sorry for the delay.
No objections from me.
This adds two pairs of wind variables for winds at the surface layer.
Note that the names here assume that the naming rule changes in #72 will be merged.