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

Remove dependency of Oceananigans on OrthogonalSphericalShellGrids #47

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

Conversation

simone-silvestri
Copy link
Collaborator

We have now a circular dependency on Oceananigans that makes it a bit difficult to develop (xref CliMA/Oceananigans.jl#3841).

If we migrate the TripolarGrid tests here, we can remove them in Oceanigans and development can proceed more smoothly.
This PR adds the tests we had in Oceananigans here if we decide to follow this route.

@glwagner
Copy link
Member

This doesn't work though. If Oceananigans development breaks test_vector_rotation we won't know until the tests here fail.

@glwagner
Copy link
Member

glwagner commented Oct 25, 2024

What you need to have is some basic OrthogonalSphericalShellGrid that allows you to unit test the Oceananigans operators without depending on OrthogonalSphericalShellGrids.jl. This is consistent with the model you are trying to achieve.

The problem with the current implementation is that the only real non-trivial OrthogonalSphericalShellGrid isTripolarGrid. Because Oceananigans does not have its own non-trivial implementation, it is unable to test all of the basic utilities that we need to support OrthogonalSphericalShellGrid , in general.

To solve this problem you need to first come up with a non-trivial basic implementation of an OrthogonalSphericalShellGrid that can live in Oceananigans. This could be anything -- even a grid loaded from file. Then you can use that grid in the Oceananigans unit tests to fully test all of the basic generic features that are needed for Oceananigans.Grids.OrthogonalSphericalShellGrid.

Then when you come to this package you need to focus on specific tests for the TripolarGrid and not put tests here that belong in Oceanangians.

These considerations are generic and apply to designing any "extension package" to some base code. You can take a look at the biogeochemistry tests for some inspiration. We have implemented a few basic bgc models for the purpose of testing, which ensures that dependent packages like OceanBioME and ClimaOceanBiogeochemistry can rely on the biogeochemistry interface.

# purely zonal flow in the extrensic frame (v ≈ 0)
@test all(on_architecture(CPU(), interior(vₑ)) .≈ 0.5)
@test all(on_architecture(CPU(), interior(uₑ)) .≈ 0.5)
end
Copy link
Member

Choose a reason for hiding this comment

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

All of these tests belong in Oceananigans. Also, they are not specific to the TripolarGrid.

@info " Testing the conversion of a vector between the Intrinsic and Extrinsic reference frame"
grid = TripolarGrid(size = (20, 20, 1), z = (0, 1))

test_vector_rotation(grid)
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to somehow grab the implementation from Oceananigans, but I'm not sure how to do that if the codes live in different repos as we have designed this code.

@simone-silvestri
Copy link
Collaborator Author

What you need to have is some basic OrthogonalSphericalShellGrid that allows you to unit test the Oceananigans operators without depending on OrthogonalSphericalShellGrids.jl. This is consistent with the model you are trying to achieve.

The problem with the current implementation is that the only real non-trivial OrthogonalSphericalShellGrid isTripolarGrid. Because Oceananigans does not have its own non-trivial implementation, it is unable to test all of the basic utilities that we need to support OrthogonalSphericalShellGrid , in general.

To solve this problem you need to first come up with a non-trivial basic implementation of an OrthogonalSphericalShellGrid that can live in Oceananigans. This could be anything -- even a grid loaded from file. Then you can use that grid in the Oceananigans unit tests to fully test all of the basic generic features that are needed for Oceananigans.Grids.OrthogonalSphericalShellGrid.

In theory, we have the CubedSpherePanelGrid that is now used for these basic tests. I am not sure this is quite enough, but it can be a start

@glwagner
Copy link
Member

In theory, we have the CubedSpherePanelGrid that is now used for these basic tests. I am not sure this is quite enough, but it can be a start

For sure, the only problem is that it does not support periodic boundaries.

Previously, I proposed developing a lat lon grid that was rotated or shifted relative to the geographic coordinate system. Perhaps with a rotated lat-lon grid + cubed sphere panel, one has something that could be used for testing.

Another possibility is to save down the metrics for a very coarse tripolar grid and use that.

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.

2 participants