-
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
Mistake in stretching matrix in multilayerqg.jl #339
Comments
Hm... trying to remember. Does the division with reference density implies that the Boussinesq approximation holds? |
The discussion #325 is probably relevant. The discussion was pointing out two things: a sign error (we fixed in #329) but also pointing out that having the denominator in the reduced gravities vary with layer brings problems in setting up an Eady problem. We should resume the discussion. Perhaps we close this issue and continue chatting in that discussion and when we converge on what to do we should open a PR and fix it? |
Sorry @navidcy, I hadn't seen this extensive discussion! We originally picked up this mistake in pyqg as Matt Lobo seems to have as well - ha! Happy to chat on that discussion. I believe that yes the reference density comes from Boussinesq. |
No worries! |
hi @mpudig, don't we still need to resolve whether we divide with |
Hi @navidcy, sorry for the slow uptake! Is the conclusion from discussion #325 that we should do away with accepting
If instead we do want to keep Let me know what you think and happy to make changes as you see fit! |
Hi,
We (Shafer Smith's group at NYU) noticed a possible typo in how the reduced gravities are defined in
multilayerqg
(https://github.com/FourierFlows/GeophysicalFlows.jl/blob/main/src/multilayerqg.jl#L355).It is our understanding that in the multi-layer QG equations, one should divide by a reference density in the reduced gravities (see Vallis, 2017 p.186), rather than by the density of each layer as is currently done in
multilayerqg
(as well as in its documentation).@navidcy, could you comment on whether this is in fact a typo or if there was a reason behind this choice for the reduced gravities?
Thanks!
The text was updated successfully, but these errors were encountered: