Skip to content

Conversation

@jrocha22
Copy link

@jrocha22 jrocha22 commented Jul 11, 2024

Description

PR to add a ConvertToStructuredGrid executable to alter files to be in structured lat/lon grid for easier post processing.
CMakeLists.txt file edited to run executable

Issue(s) addressed

No associated issue number but resolves grid difficulties that might arise from further processing of files

Testing

New executable tested with twelve different high resolution (1440x1080x75) files and two low resolution (72x35x25) files from MOM6. Variables pulled were sea surface temperature and sea water potential temperature. All files ran through with no issues.
Executable tested with Hercules.
Are the changes covered by ctests? No (If not, tests should be added first.)

@CoryMartin-NOAA
Copy link

@jrocha22 can you modify the PR description to fill out the requested items (describe what this adds, how it was tested, etc.)

@CoryMartin-NOAA
Copy link

@jrocha22 we should talk about possibly adding a test that exercises this, if @travissluka wants us to add one

@travissluka
Copy link
Contributor

@jrocha22 we should talk about possibly adding a test that exercises this, if @travissluka wants us to add one

yes, please add a ctest using one of the low res 5 deg files in soca, @jrocha22 let me know if you need guidance on how to add a ctest to soca.

side note: wow, I'm surprised that works with such little code, good job model interface team! (@fmahebert )

@jrocha22
Copy link
Author

@jrocha22 we should talk about possibly adding a test that exercises this, if @travissluka wants us to add one

yes, please add a ctest using one of the low res 5 deg files in soca, @jrocha22 let me know if you need guidance on how to add a ctest to soca.

side note: wow, I'm surprised that works with such little code, good job model interface team! (@fmahebert )

@travissluka some ctest guidance would be great! thank you

@travissluka travissluka self-requested a review July 18, 2024 15:23
Copy link
Contributor

@travissluka travissluka left a comment

Choose a reason for hiding this comment

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

Sorry, I lost track of time, I meant to send you this last week:

For adding a ctest, take a look at the soca/test/CMakeLists.txt file. Take a look at the hofx_3d test for example. Do a search in that file and you'll see hofx_3d in 3 places:

  1. The test is added with a soca_add_test command here
    soca_add_test( NAME hofx_3d
  2. The input yaml file has to have the same name as the test. In this case testinput/hofx_3d.yaml and it is added to the cmakelist file here
    testinput/hofx_3d.yml
  3. and last is the output reference file testref/hofx_3d.yaml added here
    testref/hofx_3d.test
    This file is generated by the test itself, and placed in the testoutput/ directory, I just copy over the file from testoutput/ to testref/, and the test should pass the next time you run it.

If you open testinput/hofx_3d.yaml, at the very end you'll see the output and reference files mentioned in 2 and 3 above, here

I hope that helps

@CoryMartin-NOAA
Copy link

Thanks @travissluka , @jrocha22 's summer with us is coming to a close, but either myself or @CatherineThomas-NOAA will help get this over the finish line with a test if Juliette is unable to

@travissluka travissluka marked this pull request as draft October 30, 2024 20:35
@travissluka
Copy link
Contributor

closing stale PR. can be re-opened if needed

@guillaumevernieres
Copy link
Contributor

guillaumevernieres commented Dec 12, 2025

@jrocha22 : Thanks for adding this feature! I added a test to your branch but I don't have the permissions to PR in your fork of soca. Can you give me temporary write permissions? I can also just close this and resubmit a PR with your changes.

@guillaumevernieres
Copy link
Contributor

@jrocha22 : Thanks for adding this feature! I added a test to your branch but I don't have the permissions to PR in your fork of soca. Can you give me temporary write permissions? I can also just close this and resubmit a PR with your changes.

I kept your commits @jrocha22 and submitted a new PR: #1212

@guillaumevernieres
Copy link
Contributor

Replaced with this #1212

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.

4 participants