-
Notifications
You must be signed in to change notification settings - Fork 32
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
Accepting buoyancy instead of density in MultiLayerQG
module
#343
Conversation
Thanks for this! I'll try to get to this sometime this week! |
β = 0.0, # y-gradient of Coriolis parameter | ||
U = zeros(nlayers), # imposed zonal flow U(y) in each layer | ||
H = 1/nlayers * ones(nlayers), # rest fluid height of each layer | ||
b = -(1 .+ 1/nlayers * Array{Float64}(0:nlayers-1)), # Boussinesq buoyancy of each layer |
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.
any reasoning behind this default b
?
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.
is this what corresponds to the previous default ρ?
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 think the two coincide if ρ0 = ρ[n], right?
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.
For the two-layer model this choice of default
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! nice job!
MultiLayerQG
module
@mpudig you should have merge rights now, right? if you are happy merge whenever. |
@mpudig I'll merge |
@mpudig welcome to GeophysicalFlows.jl contributors! |
@navidcy Apologies for the delay, I was making the long trip back to Australia. Thanks for merging!! |
This pull request addresses issue #339 and discussion #325.
The conclusion from the discussion was that we should accept buoyancy as an input instead of density in
multilayerqg
. The density array was used only to calculate the reduced gravity array. Previously, the derived reduced gravity array resulted in spurious internal PV gradients as discussed extensively in discussion #325. It also implied that the Boussinesq approximation did not hold, which is implicitly assumed in the form of the layered QG equations solved here.This pull request has changed the input field of$\rho$ , the density, to $b = -g \frac{\delta \rho}{\rho_0}$ , the Boussinesq buoyancy, where $|\delta \rho| \ll \rho_0$ . The reduced gravity is now derived directly from $b$ , https://github.com/mpudig/GeophysicalFlows.jl/blob/density_to_buoyancy/src/multilayerqg.jl#L348. I have set the change in buoyancy to scale like 1/nlayers with the other default parameters as they are in
multilayerqg
frommultilayerqg
– I believe this is consistent with the other nondimensional parameter values and an order 1 Burger number.