Skip to content
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

Pre-allocate grid-point pressure values and use consistent naming #91

Closed
white-alistair opened this issue Jun 13, 2022 · 5 comments
Closed
Assignees
Labels
naming 👶 Maybe there's a better name for it?

Comments

@white-alistair
Copy link
Member

This is a left over from this comment on #82

@white-alistair white-alistair self-assigned this Jun 13, 2022
@milankl
Copy link
Member

milankl commented Jun 16, 2022

On this note, for the shallow water model, I've decided to use the interface displacement η as prognostic variable, so the instantaneous layer thickness h = η + H, whereby H is the thickness at rest and H = H₀ - orography and H₀ a global constant, something like 8-12km probably. Having said that, I don't want to redefine the variable names in PrognosticVariables for the different models, so I'll be using pres (or even pres_log if we decide to redefine it) for η. This is probably an argument to keep this variable name quite generic (I vote for pres, as this has a generic meaning, whether you think about logarithm of surface pressure or interface displacement).

@white-alistair
Copy link
Member Author

@milankl picking this up again. How about the following approach:

  1. Pre-allocate a new grid variable pres_exp_grid in the GridVariables struct (the name is potentially misleading, but I think all of the options are suboptimal in their own way, so I'd be happy to go with your preference from the previous comment).
  2. In tendencies_parametrizations.jl, do pres_exp_grid = exp.(pres_grid), i.e. before any of the parametrizations are calculated, since this quantity is used by several of them. I'm not 100% this is the most logical place to do this, but as far as I know we only use this in the parametrizations.

Let me know what you think!

@milankl
Copy link
Member

milankl commented Jul 12, 2022

Is pres_grid (i.e. the logarithm of surface pressure on the grid) even used for the tendencies? If not, we could just have pres_grid which is first transformed from spectral pres and then calculate the exponent in-place? Then pres would always be the logarithm of surface pressure in spectral space and pres_grid the surface pressure (hPa) in grid-point space.

@milankl
Copy link
Member

milankl commented Jul 12, 2022

I agree that we should have one place where the exp is calculated as this is expensive (usually taking several times more cycles than +,-,*,/ etc) but maybe we don't have to allocate another array but can do it in-place like

for i in eachindex(pres_grid)
    pres_grid[i] = exp(pres_grid[i])
end

@milankl milankl added the naming 👶 Maybe there's a better name for it? label Jul 13, 2022
@white-alistair
Copy link
Member Author

As far as I can see it's only used for the physics, so yeah I think this works. I'll just comment it clearly so newcomers aren't confused :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
naming 👶 Maybe there's a better name for it?
Projects
None yet
Development

No branches or pull requests

2 participants