Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add custom forcing velocity to particles #3396
base: main
Are you sure you want to change the base?
Add custom forcing velocity to particles #3396
Changes from 7 commits
95f68a1
a0092d6
fc334a6
29e5b07
2143588
d02326d
3b6f574
56cd1f9
ed5c801
06fbbd9
64b409d
0ce23f5
e2bb2cf
788d096
68406cd
8c4a063
6020ed1
e7c5ece
3d8f8f9
3c691b3
b615847
0420f55
6ac1f39
0c8b340
c015b8b
f9e1093
c2b5d05
a0f8fe2
901aa5f
2e1080e
e17c93e
0ae0bb4
5dd7ed1
4682818
00a1627
ca51fab
a6bc090
a054014
446f0e9
9055c28
7df6229
a867e55
48602bf
8ab3eef
32e0c98
c21a5d7
cb39226
dd548a3
59b0aae
49e05b8
e8a8224
48ec4e6
21396d0
83869ba
8bff074
7ed8dc3
c0f4037
46c411a
2d67753
778dd91
1841b2c
c7daaad
0b6fcc8
7f5ddf7
16c4816
f40ab1f
a5aa48d
16f22bb
dd89374
370fab7
c967882
789d79e
2f4d470
bf3b6f3
dd384b6
dfdcf2c
a73e845
3a7f662
54ce501
b53738a
959ddab
b78c327
e76406d
b24448c
c5540c0
219e835
bfcfaa3
8234689
96d970b
aa7f47f
aa51f14
bc9d6f3
fcdcd1b
9a837da
f88eb20
06739c3
fcd03f0
edc3610
fc7d129
b4eda3d
99a64f9
d650c94
2cb0c3e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
we cannot pass the
model
to a GPU kernel because we don't have anadapt
function for the model.You should extract the fields that you need and pass those to the kernel individually
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 see. Is
particles
fine? What do you think theParticleAdvectiveForcing
arguments should contain? I'm imagining(particles, fluid_velocities, background_velocities, tracers, Δt)
perhaps.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.
sounds good! maybe also auxiliary fields to allow customization?
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.
and the buoyancy model to allow for buoyant particles
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.
Put the same stuff in it that we put into the tendency kernel forcing functions.
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.
There is no
i, j, k
though because this is particles right? The equivalent isp
, the particle index. So the discrete form arguments would bep, grid, clock, model_fields
.Shouldn't you multiply this by
Δt
(because its forward Euler forcing)? I don't think you need to pass inΔt
.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.
The forcings are called here:
Oceananigans.jl/src/Models/NonhydrostaticModels/nonhydrostatic_tendency_kernel_functions.jl
Line 80 in 76c32f6
for example.
DiscreteForcing
is a simple wrapper function that allows users to also store "parameters" in the forcing object.Eventually, it makes sense to use the same here, and have "discreet form" and "continuous form" for
ParticleForcing
. It's up to you whether you want to go all the way. Those wrappers are pure convenience and do not add functionality.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.
Should we call it simply
ParticleForcing
? It's true that this is a forcing on particle position and is therefore, in some sense, "advective". But I'm not sure it actually helps clarify how the forcing is used.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.
We would need
i, j, k
formodel_fields
unless we want to interpolate them and pass themThere 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.
Hmm, but what would
i, j, k
be? I guess to usemodel_fields
, the user here would have to callinterpolate
manually, and they would also need to know the staggered location...I do like the idea of interpolating the model fields, but to put that in we probably want to design an API where the interpolation only happens if the users want to use those fields, eg similar to how we have
field_dependencies
inContinuousForcing
. But as a first pass, we can simply avoid interpolating ifisnothing(particle_forcing)
.