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

Momentum dynamics validation on the CubedSphere #3306

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

Conversation

navidcy
Copy link
Collaborator

@navidcy navidcy commented Oct 4, 2023

with @glwagner

Closes #3265

@navidcy
Copy link
Collaborator Author

navidcy commented Oct 4, 2023

it blows up at some point...

cubed_sphere_momentum_dynamics.mp4

I even reduced the tilmestep but not sure if it's that or corner-related artifacts

@glwagner
Copy link
Member

glwagner commented Oct 4, 2023

it blows up at some point...

cubed_sphere_momentum_dynamics.mp4
I even reduced the tilmestep but not sure if it's that or corner-related artifacts

Might help to plot vorticity. Are we using WENO or second order momentum advection?

@navidcy
Copy link
Collaborator Author

navidcy commented Oct 4, 2023

it blows up at some point...
cubed_sphere_momentum_dynamics.mp4
I even reduced the tilmestep but not sure if it's that or corner-related artifacts

Might help to plot vorticity. Are we using WENO or second order momentum advection?

I didn't prescribe anything so probably 2nd order.

@glwagner
Copy link
Member

glwagner commented Oct 4, 2023

it blows up at some point...
cubed_sphere_momentum_dynamics.mp4
I even reduced the tilmestep but not sure if it's that or corner-related artifacts

Might help to plot vorticity. Are we using WENO or second order momentum advection?

I didn't prescribe anything so probably 2nd order.

Could be the culprit if we don't have viscosity, how about with viscosity or WENO?

@navidcy
Copy link
Collaborator Author

navidcy commented Oct 5, 2023

@siddharthabishnu, can you commit the steady-state validation script in this PR from #3302? We were trying to ensure that double halo passing occurs for velocities etc and these are probably also needed for #3302 so let's have both validation scripts in one place so we don't do double job?

@siddharthabishnu
Copy link
Contributor

siddharthabishnu commented Oct 5, 2023

@siddharthabishnu, can you commit the steady-state validation script in this PR from #3302? We were trying to ensure that double halo passing occurs for velocities etc and these are probably also needed for #3302 so let's have both validation scripts in one place so we don't do double job?

Done. 😊

@glwagner
Copy link
Member

glwagner commented Oct 7, 2023

Any progress visualizing vorticity?

@navidcy
Copy link
Collaborator Author

navidcy commented Oct 8, 2023

Yes. I'll post a script. But it once again demonstrated that halos are not filled properly for face-face fields so I'm trying to deal with #3280 first!

@glwagner
Copy link
Member

Why do we need to fill halos for face-face?

Comment on lines +248 to +251
if size(data)[3] == 1 && first.(axes(data))[3] == grid.Nz + 1 # take ssh into consideration
parent_indices = collect(parent_indices)
parent_indices[3] = 1:1
end
Copy link
Collaborator Author

@navidcy navidcy Apr 17, 2024

Choose a reason for hiding this comment

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

this reads like a hack..

also, what's "ssh" mentioned in the comment?

what if size(data)[2] == 1... then nothing needs to happen?

Copy link
Contributor

@siddharthabishnu siddharthabishnu May 1, 2024

Choose a reason for hiding this comment

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

Yes, it is a temporary hack. I have outlined the issue arising in the absence of this hack in issue #3572 and created PR #3573 to resolve it. In the comment, ssh refers to fields like ssh defined on the xy plane, for which the parent array takes the z-halo into consideration even though it does not exist. In case of a field like the meridional average of a scalar quantity which is defined on the xz plane, the same issue arises without an equivalent hack.

Comment on lines +97 to +98
#- for Forward-Backward+AB2 hack to work:
fill_halo_regions!(model.free_surface.η)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what if model doesn't have a free surface?

end
end
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
end
end

end
end
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
end
end

Comment on lines 35 to 43
#=
@inline Γᶠᶠᶜ(i, j, k, grid::OrthogonalSphericalShellGrid, u, v) =
ifelse(on_south_west_corner(i, j, grid) | on_north_west_corner(i, j, grid),
Δy_qᶜᶠᶜ(i, j, k, grid, v) - Δx_qᶠᶜᶜ(i, j, k, grid, u) + Δx_qᶠᶜᶜ(i, j-1, k, grid, u),
ifelse(on_south_east_corner(i, j, grid) | on_north_east_corner(i, j, grid),
- Δy_qᶜᶠᶜ(i-1, j, k, grid, v) - Δx_qᶠᶜᶜ(i, j, k, grid, u) + Δx_qᶠᶜᶜ(i, j-1, k, grid, u),
δxᶠᶜᶜ(i, j, k, grid, Δy_qᶜᶠᶜ, v) - δyᶜᶠᶜ(i, j, k, grid, Δx_qᶠᶜᶜ, u)))
=#

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this old/deprecated code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I will delete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants