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

Support for user-defined forcing functions. #85

Merged
merged 5 commits into from
Feb 27, 2019
Merged

Conversation

ali-ramadhan
Copy link
Member

Just hacked something together that allows for user-defined forcing functions for the CPU. Have not tested on the GPU yet.

Basically there's a struct Forcing that stores the user-defined forcing functions. It will replace the old ForcingFields struct. See examples/deep_convection_3d.jl for how I switched to using a forcing function for T to enforce a cooling surface heat flux.

A big issue is that the current implementation slows down the time stepping by a factor of 2-3x. So we'll have to figure out why before merging.

The function must have a signature like F(u, v, w, T, S, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) right now so this won't produce a nice solution as we will have to figure out #59 before the function signature can look as nice as surface_cooling_disk(grid, velocities, tracers, i, j, k). This is work for another branch.

Will keep working on this before merging. Just wanted to start something.

Resolves #73

@ali-ramadhan ali-ramadhan added the feature 🌟 Something new and shiny label Feb 26, 2019
@glwagner
Copy link
Member

Should we just try to resolve #59 now?

src/forcing.jl Outdated
S::TS
end

@inline zero_func(u, v, w, T, S, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it slower to write @inline zero_func(args...) = 0? This is less explicit, but does not require maintenance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@inbounds Gu[i, j, k] = -u∇u(u, v, w, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) + fCor*avg_xy(v, Nx, Ny, i, j, k) - δx_c2f(pHY′, Nx, i, j, k) / (Δx * ρ₀) + 𝜈∇²u(u, 𝜈h, 𝜈v, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k)
@inbounds Gv[i, j, k] = -u∇v(u, v, w, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) - fCor*avg_xy(u, Nx, Ny, i, j, k) - δy_c2f(pHY′, Ny, i, j, k) / (Δy * ρ₀) + 𝜈∇²v(v, 𝜈h, 𝜈v, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k)
@inbounds Gw[i, j, k] = -u∇w(u, v, w, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) + 𝜈∇²w(w, 𝜈h, 𝜈v, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k)
@inbounds Gu[i, j, k] = -u∇u(u, v, w, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) + fCor*avg_xy(v, Nx, Ny, i, j, k) - δx_c2f(pHY′, Nx, i, j, k) / (Δx * ρ₀) + 𝜈∇²u(u, 𝜈h, 𝜈v, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) + F.u(u, v, w, T, S, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you try to pass grid instead of Nx, Ny, Nz, Δx, Δy, Δz?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the GPU it complains that the arguments are not isbitstype :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. So both Fields and Grids will have to be worked on.

@ali-ramadhan
Copy link
Member Author

ali-ramadhan commented Feb 26, 2019

I think there are already a couple of challenges for this pull request:

  1. Implementing user-defined forcing functions without losing performance.
  2. Getting the user-defined forcing functions to work on the GPU without losing performance either.

As #59 will result in a ton of refactoring, I think it might be more appropriate to address it in a different branch after #73 is resolved, so that everything (Forcing is already isbitstype but no other code depends on it) can be converted into isbitstype.

If doing so results in large performance drops (which I think might have happened here) then it might take a while to figure out #59.

Copy link
Collaborator

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Cthulhu to see if things are inlined properly.

src/forcing.jl Outdated
@inline zero_func(u, v, w, T, S, Nx, Ny, Nz, Δx, Δy, Δz, i, j, k) = 0

function Forcing(Tu, Tv, Tw, TT, TS)
if Tu == nothing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use === when comparing against nothing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ali-ramadhan
Copy link
Member Author

Performance is fine now so merging in. Thanks @glwagner!

@ali-ramadhan ali-ramadhan merged commit 9d43e79 into master Feb 27, 2019
@ali-ramadhan ali-ramadhan deleted the forcings branch February 27, 2019 23:15
ali-ramadhan added a commit that referenced this pull request Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🌟 Something new and shiny
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants