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

read and write regular lat/lon atlas grids #102

Merged
merged 13 commits into from
Jul 18, 2024

Conversation

twsearle
Copy link
Collaborator

@twsearle twsearle commented Jul 5, 2024

Description

Add reading and writing of regular latitude/longitude grid data.

  • Move to factory pattern abstraction for AtlasIndex classes
  • Support indexing regular grids.
  • Read server can read structured grids.
  • extend Write server to structured grids.

Also includes:

  • a new low resolution synthetic dataset for the purposes of testing.
amm1 grid example
  • a new set of jedi observations in the AMM region for testing.

compare

Issue(s) addressed

Resolves #101

Dependencies

None at this time.

Impact

Enables multiple regional ocean model configurations in JOPA.

Testing checklist

twsearle added 2 commits July 5, 2024 13:20
 * Add factory pattern abstraction for AtlasIndex classes
 * Support indexing regular grids
 * Read server can read structured grids.
 * extend WriteServer to structured grids
@twsearle twsearle marked this pull request as draft July 5, 2024 12:29
@twsearle
Copy link
Collaborator Author

twsearle commented Jul 5, 2024

Note, this PR is guaranteed to fail CI at this time. The change has been introduced to share and discuss issues with the results of the tests. When the tests pass, the PR will come out of draft.

@twsearle twsearle self-assigned this Jul 5, 2024
@twsearle twsearle force-pushed the feature/read-regular-lonlat branch from e5537e5 to e48ccf5 Compare July 9, 2024 15:06
@twsearle twsearle requested a review from s-good July 11, 2024 11:38
@twsearle twsearle marked this pull request as ready for review July 11, 2024 11:38
@twsearle twsearle force-pushed the feature/read-regular-lonlat branch from 9435cc3 to 2a85691 Compare July 11, 2024 11:39
@twsearle
Copy link
Collaborator Author

I took out the failing integration test and this looks to pass now so I have put it in review. I will shortly add links to sith comparison workflow runs for this change.

@twsearle
Copy link
Collaborator Author

Looks like there might be some issues with AtlasIndex.h. I am getting some issues with orcamodel::OrcaIndexToBufferIndex::ij in the sith comparisons, so I won't merge until those issues are resolved. I think I could still benefit from a review though

@twsearle twsearle requested a review from odlomax July 12, 2024 15:20
@twsearle twsearle force-pushed the feature/read-regular-lonlat branch from e006a95 to 3755d37 Compare July 15, 2024 09:36
@twsearle
Copy link
Collaborator Author

I think I have fixed the issue with the testing - I was accidentally using the atlas v0.38.1 KGO for my sith comparison tests. I also found the issue with the AMM tests - I was reading in the wrong observation file input! Apologies it took me so long to spot this (a lot of uncessary print statements) but on the bright side, I have interpolated a couple of the observations by hand so I am now quite convinced that this is working correctly.

Copy link
Collaborator

@s-good s-good left a comment

Choose a reason for hiding this comment

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

Looks great, thanks. I just left a few comments for you to consider.

src/orca-jedi/nemo_io/AtlasIndex.h Show resolved Hide resolved
src/orca-jedi/nemo_io/AtlasIndex.h Show resolved Hide resolved
src/orca-jedi/nemo_io/AtlasIndex.h Show resolved Hide resolved
src/tests/orca-jedi/test_interpolator.cc Show resolved Hide resolved
@twsearle twsearle requested a review from s-good July 17, 2024 18:39
Copy link
Collaborator

@s-good s-good left a comment

Choose a reason for hiding this comment

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

Thanks for making the change and responding to my comments.

@twsearle twsearle merged commit a7295bf into develop Jul 18, 2024
2 checks passed
@twsearle twsearle deleted the feature/read-regular-lonlat branch July 29, 2024 08:26
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.

Read and write support for regional regular longitude/latitude model grids
2 participants