ACHMartin / seastar_project

4 stars 0 forks source link

Standardise variable names for velocity components #240

Closed DavidMcCann-NOC closed 11 months ago

DavidMcCann-NOC commented 1 year ago

Standardize the variable names for velocity components (currents and wind) across all functions so that we have: Currents: CurrentU CurrentV Wind: WindU WindV

Also check and update all docstrings

ACHMartin commented 1 year ago

Also change CurrentMagnitude to CurrentVelocity

ACHMartin commented 1 year ago

we also have vis_u, vis_v, vis_speed, vis_direction which correspond to the Visible Wind (relative to the surface), on contrast to the Wind which is an absolute wind compared to solid earth. Proposition to VisU, VisV, VisSpeed, VisCurrent (not ideal...), VisibleU, ... RelativeWindU, ... RelWindU, ...

Do we move our previous WindU to AbsWindU, and our visible to RelWindU, ...

ACHMartin commented 1 year ago

This raise, lots of question as for exemple compute_nrcs is using WindSpeed and WindDirection as input. Same for compute_wasv I guess.

Then should we move all these functions to RelativeWindSpeed, ...

ACHMartin commented 1 year ago

my preference would be to change WindSpeed -> AbsoluteWindSpeed or RelativeWindSpeed depending of the context. I would prefer not to keep the standard "WindSpeed" without precision as it would certainly trigger invisible bug

DavidMcCann-NOC commented 1 year ago

In my view 'absolute' has a defined mathematical meaning, i.e. sqrt( Windspeed **2 ), so as long as AbsoluteWindSpeed satisfies this then yes, certainly. 'relative' denotes being relative to something else, and as its attached to 'speed' then RelativeWindSpeed would denote the wind speed relative to a different speed or reference frame. In aeronautics they use 'true' and 'indicated' to basically mean 'absolute' and 'relative', with the speed its relative to being the sensor (aircraft).

If the measurement is from, or simulates, a fixed sensor or reference frame (i.e., Eulerian) then in my view its Absolute. If its from a moving sensor or reference frame and is un-corrected (i.e., true Lagrangian) and/or the product or difference of another velocity vector then its Relative

DavidMcCann-NOC commented 1 year ago

we also have vis_u, vis_v, vis_speed, vis_direction which correspond to the Visible Wind (relative to the surface), on contrast to the Wind which is an absolute wind compared to solid earth.

I really should look further up shouldn't I! I'm not sure I'm up for visible wind, but maybe thats because i've never used that terminology before and to me it sound like i can see the wind!

But if its standard in scatterometry, meterology etc then would be happy with that definition.

Perhaps finding a standard that other people use and following that might help? Personally I'd probably get along with any variable change as long as its documented

ACHMartin commented 1 year ago

Related to the mathematical definition, it will work for AbsoluteWindSpeed, but not for AbsoluteWindU or AbsoluteWindV which would be positive and negative. For AbsoluteWindDirection, it is modulo true...

We only touch these issues in scatterometry as current are typically very small 0.5m/s. So far, no consensus I am aware of. FixedEarthWind, EarthRelativeWind versus SurfaceRelativeWind ... what a nightmare

DavidMcCann-NOC commented 1 year ago

A right can of worms!

Well I certainly like EarthRelative as it directly denotes the measurement is relative to the (spinning?) Earth. If we want to avoid using 'Absolute' for U and V velocity components (and I would say thats important) then EarthRelativeWindU, while long, tells me that its a velocity component of the wind, relative to the spinning Earth. In an equation I'd probably use subscripts like Uwre but thats not transparent enough for a program eh.

I like EarthRelative, so the choice for the 'other' reference frame would probably best be SurfaceRelative?

DavidMcCann-NOC commented 1 year ago

Once we have a consensus, it would be good to make a table here of the variable names and definitions so I can refer to it when going through and changing the code

ACHMartin commented 1 year ago

After, your remark, scatterometer winds are called Ocean Surface Vector Wind (OSVW). So we should probably keep the OceanSurface prefix. Then the other one will be EarthRelative

DavidMcCann-NOC commented 1 year ago

This is good - so currently we have:

Relative to the ocean surface ('standard' oceanographic variables):

Current velocity = OceanSurfaceCurrentVelocity Current Direction = OceanSurfaceCurrentDirection Current U component = OceanSurfaceCurrentU Current V component = `OceanSurfaceCurrentV

Wind velocity = OceanSurfaceWindVelocity Wind Direction = OceanSurfaceWindDirection Wind U component = OceanSurfaceWindU Wind V component = OceanSurfaceWindV

Relative to the spinning Earth:

Current velocity = EarthRelativeCurrentVelocity Current Direction = EarthRelativeCurrentDirection Current U component = EarthRelativeCurrentU Current V component = EarthRelativeCurrentV

Wind velocity = EarthRelativeWindVelocity Wind Direction = EarthRelativeWindDirection Wind U component = EarthRelativeWindU Wind V component = EarthRelativeWindV

Do we want to add Total to make it TotalSurfaceCurrentVelocity to align with the literature? As in with WASV removed?

ACHMartin commented 1 year ago

we only need it for the wind, not for the current. The current is always relative to the Earth, no need to specify it, we can stick with CurrentVelocity.

ACHMartin commented 11 months ago

This issue is closed with commit #270