Skip to content

Adds few utilities for stretched coordinates + Docs/Grids section on how to use these utils #4623

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

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

Conversation

navidcy
Copy link
Member

@navidcy navidcy commented Jun 28, 2025

Originally developed at CliMA/ClimaOcean.jl#565

This PR brings utilities for constructing variably-spaced coordinates from ClimaOcean to Oceananigans. Namely ExponentialCoordinate and ConstantToStretchedCoordinate. The main features implemented in this PR are in src/Grids/coordinate_utils.jl + the Docs/Grids corresponding section that demonstrates their functionality.

The PR also makes minor several fixes in other docstrings.

Docs preview at https://clima.github.io/OceananigansDocumentation/previews/PR4623/grids/#Coordinate-helper-utilities

@navidcy navidcy requested a review from glwagner June 28, 2025 02:58
@glwagner
Copy link
Member

I suggest we merge this and leave the StretchedCoordinate to be vertical-coordinate specific in ClimaOcean?

What's the rationale? I think there are disadvantages to overlapping scope between the packages so it'd nice to clarify what-belongs-where. I think ClimaOcean concerns should be focused on OceanSeaIceModel whereas anything to do with grid generation is more fundamental and seems to be an Oceananigans concern?

@@ -0,0 +1,111 @@
struct ExponentialCoordinate <: Function
Copy link
Member

Choose a reason for hiding this comment

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

we could relax the Function constraint in this PR as well

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to have a unique type otherwise it's not picked up by validate_dimension_specification method.
I defined a new abstract type.

@navidcy
Copy link
Member Author

navidcy commented Jul 1, 2025

I agree with the comments. I'll make the changes and ping you!

@navidcy
Copy link
Member Author

navidcy commented Jul 1, 2025

I suggest we merge this and leave the StretchedCoordinate to be vertical-coordinate specific in ClimaOcean?

What's the rationale? I think there are disadvantages to overlapping scope between the packages so it'd nice to clarify what-belongs-where. I think ClimaOcean concerns should be focused on OceanSeaIceModel whereas anything to do with grid generation is more fundamental and seems to be an Oceananigans concern?

Yes, feels natural.

@navidcy navidcy changed the title Add ExponentialCoordinate + Docs/Grids section on Stretched Coordinates Adds few utilities for stretched coordinates + Docs/Grids section on how to use these utils Jul 5, 2025
@navidcy
Copy link
Member Author

navidcy commented Jul 5, 2025

@glwagner, I brought everything here and renamed it to coordinate_utils.jl. Have another look. When merged, I'll delete from ClimaOcean.

@navidcy navidcy requested a review from glwagner July 5, 2025 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants