SurfaceFluxes.jl icon indicating copy to clipboard operation
SurfaceFluxes.jl copied to clipboard

Type system is overcomplicated

Open glwagner opened this issue 1 year ago • 3 comments

I'm wondering if the code is more complicated than it needs to be. There are three types for each universal function, eg BusingerParams, BusingerType, and then Businger.

The "*Type" is superfluous with the *Params types, since each Type is simply related to each Params adding no additional information:

https://github.com/CliMA/SurfaceFluxes.jl/blob/f83dc8642b032184c3b31b2c444ed1a6f43168f1/src/UniversalFunctions.jl#L1113-L1118

I propose refactoring the code to simplify the type structure by combining the concepts of "params" and "type".

I also cannot see the purpose of the types which are currently called Businger, Gryanik, etc. These types are distinct from the "params" and "type" objects by providing a place to store the Monin-Obukhov length. But these objects to not compute this length; it's computed externally. And the length stored there is never actually used outside of the scope where it is computed. In other words, the only time we access the length L is a few lines after it is computed (here, L_MO is an input argument to the function):

https://github.com/CliMA/SurfaceFluxes.jl/blob/f83dc8642b032184c3b31b2c444ed1a6f43168f1/src/SurfaceFluxes.jl#L806-L817

If there is no specific purpose for Businger, Gryanik then they should be eliminated. Note also that these types are all identical, so even if we want to keep them, there only needs to be one type that holds on to L.

In summary, I propose the following changes:

  1. Eliminate the types that are typically notated as uf: Businger, Gryanik, etc
  2. Eliminate the "*Type" constructs: BusingerType, etc
  3. Use the *Params types for dispatch, where Businger and BusingerType are now used
  4. Remove the Params suffix from the existing types to make the lexicon more concise and clear, reflecting the fact that these types both control dispatch and also hold parameter values. Eg BusingerParams should simply be called Businger (or BusingerUniversalFunction to be more verbose).

The first 3 changes are purely internal refactoring, while the last changes the user API.

glwagner avatar Jan 10 '24 02:01 glwagner