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

Resize bottom field when using with_halo on ImmersedBoundaryGrid #3138

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

Conversation

simone-silvestri
Copy link
Collaborator

closes #3137

@glwagner glwagner changed the title resize bottom field when using with_halo on ImmersedBoundaryGrid Resize bottom field when using with_halo on ImmersedBoundaryGrid Jul 11, 2023
using Oceananigans.Fields: location, ZReducedField, Field

instantiate(T::Type) = T()
instantiate(t) = t
Copy link
Member

Choose a reason for hiding this comment

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

It's ok to redefine instantiate here, since it makes the code easier to read (ie a case where "Write Everything Twice" (WET) is better than "Don't Repeat Yourself" (DRY). Just FYI for the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just want to make this pass first
https://buildkite.com/clima/oceananigans/builds/11941#01890352-3b10-49b6-98a3-0745c630a86c
then of course I agree on the comment

Copy link
Member

Choose a reason for hiding this comment

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

Made a comment below to help

@tomchor
Copy link
Collaborator

tomchor commented Jan 26, 2024

@simone-silvestri the issue which this was meant to close is now closed. (See #3137 (comment))

Should we close this PR as well?

@simone-silvestri
Copy link
Collaborator Author

Does that PR also handle distributed grids? Because that was a problem that should be solved by this PR. If that is the case, we can close this one

@tomchor
Copy link
Collaborator

tomchor commented Jan 26, 2024

Does that PR also handle distributed grids?

That issue had nothing to do with anything distributed. So if this fixes some issue with distributed grid then maybe it should be kept. Although it seems stale... :(

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

Successfully merging this pull request may close these issues.

Building WENO advection with immersed grid fails due to grid size
3 participants