Closed milankl closed 2 months ago
That's great! Now it looks very close to the math expression.
Yes that's intended but I'm still struggling to make our functions consistent with respect to radius and or coslat scaling. Maybe you have ideas/opinions on that, for context
Now for the user-facing functions I keep facing questions of
Feel free to let us know what you would find most intuitive and clear or what you think a general user would expect?
Yes, I see your "struggling" too. When I design some gradient
APIs in python, I need to add a kwarg as
grdx, grdy = gradient(v, Rearth=6371e3)
so that users could change the default radius of earth (like scaling or not here, they may choose a slightly value of Rearth
).
In such function-style APIs, one needs this "extra" kwarg in every function (gradient, laplacian, divergence etc.), which is bit annoying. So a solution would be define an Application
class (OOP-style), and remember users' choice at the start when Application
is initialized. I do this sometimes. I also tried to define some internal function with long list of args so that it is flexible for developers (e.g., grdx, grdy, grdz = __gradient(v, dims=['x', 'y', 'x'], Rearth=6371e3)
), and then exposed them to users with pre-defined default parameters (e.g., grdx, grdy = hgrad(v, Rearth=6371e3)
, which uses __gradient
) so that many details are hidden.
I feel the case here is a bit similar. For users with gridded data, I feel like hidding all the scaling is better, but also allow users to change Rearth
, as users do not need to know the details of the spectral transform. For (advanced) users dealing with spectral data, hidding all scaling is not possible, as cos-scaling is intrinsic in spectral method (for velocity, and users need to know this). So let users take this responsibility should be fine. Although this cos/radius scaling should not be an overhead for diagnostic purpose, it is much better to store the pre-computed spectral coefficient for faster calculation (this seems to fit the OOP-style).
As one of the users, I currently prefer:
Glad you share your thoughts, but I feel like never find a perfect solution (╥﹏╥).
Awesome thanks for the input, I've put this example into the docs: https://speedyweather.github.io/SpeedyWeather.jl/previews/PR523/gradients/
All methods exposed to the user (the allocating ones without the in-place !
) take now an optional keyword argument radius
so that dGdx, dGdy = ∇(G, radius=6.371e6)
would compute the gradient on Earth in 1/meters. I don't like the idea of having a somewhat global radius one can set as it also violates the (Julia / functional programming) philosophy to avoid globals as much as possible to have functions that only depend on their arguments, not on some "spooky action at a distance". By default the radius is 1.
Grid in, grid out and then the coslat-scaling is also directly implemented, when providing spectral fields as inputs the scaling isn't done as the gradient operators (all acting on spectral fields) would require another two transforms to account for scaling. I think that's a consistent logic we can probably stick to (grid = with scaling, spectral = without).
I've (hopefully all correct) added these specifics to every docstring too, e.g. ?∇
, ?divergence
etc.
@miniufo as discussed some convience functions to compute the gradient of a
LowerTriangularMatrix
orAbstractGrid
. Also other operators∇ ∇² ∇! ∇⁻² ∇²! ∇⁻²!
are now all exported. Gradient can be used asas always, radius scaling is omitted, so you'd need to
/radius
manually. Results inMore methods in the docstring