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

Reduce parameter space for velocity tendency kernels #3924

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

simone-silvestri
Copy link
Collaborator

@simone-silvestri simone-silvestri commented Nov 14, 2024

If we use complicated forcings in the model, for example the ECCORestoring type we use in ClimaOcean, we could incur in parameter space by passing all the forcings in the kernels, see:
https://buildkite.com/clima/climaocean-examples/builds/942#01932566-3781-41ab-9464-031b0bfcf562
and
CliMA/ClimaOcean.jl#244

This PR should reduce the parameter space in those cases by passing only the u and v specific forcings.

@navidcy navidcy added the GPU 👾 Where Oceananigans gets its powers from label Nov 14, 2024
@glwagner
Copy link
Member

This is nice, but can you implement this uniformally for all models? It'll be faster to do it now, rather than run into this issue in the future and be scratching our heads because we forgot about this

@simone-silvestri simone-silvestri changed the title Reduce parameter space for the u and v hydrostatic tendency kernels Reduce parameter space for velocity tendency kernels Nov 14, 2024
@simone-silvestri
Copy link
Collaborator Author

I suspect that this PR is slowing down the code. I ll investigate

@simone-silvestri simone-silvestri added the 🚨 DO NOT MERGE 🚨 IN BIG BOLD RED CAPS WITH FLASHING SIRENS label Nov 15, 2024
@simone-silvestri simone-silvestri removed the 🚨 DO NOT MERGE 🚨 IN BIG BOLD RED CAPS WITH FLASHING SIRENS label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GPU 👾 Where Oceananigans gets its powers from
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants