-
Notifications
You must be signed in to change notification settings - Fork 12
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
Update vector hyperdiffusion to act on full vectors #1654
Comments
@tapios, let me know if this is how you think we should address the vector hyperdiffusion issue or if I am misunderstanding the problem. It seems that after the operation is done, we just need to extract the correct components for the tendencies. |
cc. @akshaysridhar |
We need some more detail here:
|
Thank you for the clarifications.
In the code, we use the spectral element differential operators only for hyperdiffusion (e.g., there should only be derivatives wrt the first two generalized coordinates here).
100%. We will diffuse The output from this operation would be a Let me know if this matches your idea. |
As I understand correctly we want to add hyperdiffusion for the w (u_3) component. We can do this by seeing w as a scalar and have to apply the machinery for scalar hyperdiffusion to w with the right metric terms (have to be interpolated from cell to faces) |
Technical comment: ClimaCore.jl does not provide a horizontal curl (spectral element operator) that acts on Covariant123Vectors. A simple, workaround in pseudocode may look like this:
cc. @akshaysridhar |
Expanding this following REPL tests with existing ClimaCore Operators - Field values are truncated in the MWE below (given u_bar is accessible from cache variables): χ denote intermediate variables in this demonstration.
i.e. the ClimaCore block contains the 2 operations we need separately (as in the "hack" above), an additional |
Ok. I will open a draft PR that builds out the "hack" first. We can spec out how we would replace the hack with a new ClimaCore operations and open a separate PR there. This will allow us to run out test cases and spec out new ClimaCore functionality at the same time. |
Noted, thanks! |
We will also compare the math to the case of treating u_3 as a scalar wrt hyperdiffusion per @OsKnoth's point. |
Is this only involving horizontal derivatives? It looks to me from https://clima.github.io/ClimaCore.jl/stable/operators/#ClimaCore.Operators.Curl that scurl also has the vertical derivatives, which we do not want here. |
I believe the docs are not correct, because in the code here https://github.com/CliMA/ClimaCore.jl/blob/77b6294fd9f55dba925b8b28bb8c5ca49de6dd68/src/Operators/spectralelement.jl#L1108 there seem to only be spectral derivatives in the 1 & 2 directions. |
Ok. It would be good opportunity then to make sure that operator names more clearly reflect what they do (e.g., we have some with h subscript for horizontal). @simonbyrne Could you please review plans here and amend where needed, so we are using the correct operators (with horizontal derivatives)? |
Technical comment:
|
Currently the vector hyperdiffusion operation in ClimaAtmos does not operate like the standard vector hyperdiffusion. This is because the current vector Laplacian used in the implementation does not act on three-dimensional vector fields.
Loosely speaking, we are currently doing this:
We should use the correct vector Laplacian by chaining together differential operators and coordinate transforms that act on three-dimensional vector fields. For example like this:
In this pseudocode, the main software issue lies with
scurl(u_bar)
because our spectral element curl operator currently does not operate onCovariant123Vector
types out of the box. The fix seems reasonably simple in ClimaCore.jl, but a hacky solution also exists.The text was updated successfully, but these errors were encountered: