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

Completing the do_cube_transform implementation in make_hgrid v2.0 #195

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

ngs333
Copy link
Contributor

@ngs333 ngs333 commented Nov 28, 2022

This PR completes the do_cube transform implementation in make_hgrid. The feature was requested in issue #179. Upon code inspection, exploration and testing we determined that only minor code changes were needed to complete its implementation.

This is an important feature which was unfinished and therefore without testing, exercise or coverage in any unit test, and so a reference unit test (# 33) was added. The unit tests compare make_hgrid horiz grids created analyticall vs created from reference FV3 output grid spec files. Additional manual testing was done and further discussion is available in issue #179.

The earlier version of this PR (#194) was closed without merging in order to facilitate complete remove the larger ( approx C128 size) reference files from github.

Also adds unit tests comparing make_hgrid horiz grids created
analyticly vs created from refrence FV3 output grid spec files.
@ngs333 ngs333 linked an issue Nov 28, 2022 that may be closed by this pull request
@ngs333 ngs333 closed this Nov 28, 2022
@ngs333
Copy link
Contributor Author

ngs333 commented Nov 28, 2022

The PR was closed to facilitate the complete removal of the very large unit test reference files. There is a replacement PR that uses smaller (C48 size) files #195 which are more convenient.

@ngs333
Copy link
Contributor Author

ngs333 commented Nov 28, 2022

Accidentally closed wrong PR. Re-opening.

@ngs333 ngs333 linked an issue Nov 28, 2022 that may be closed by this pull request
Copy link

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

@underwoo - do we want any datafiles in our test suite? If not, is there another way to make them available for CI testing within the container?

Copy link

@lharris4 lharris4 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks.

@ngs333
Copy link
Contributor Author

ngs333 commented Nov 30, 2022

@underwoo - do we want any datafiles in our test suite? If not, is there another way to make them available for CI testing within the container?

@underwoo , @bensonr
The Test33 reference dataset is six files of 132 KB each. Current size of
the t directory immediately after a git clone is 887 MB.

@ngs333
Copy link
Contributor Author

ngs333 commented Dec 1, 2022

@underwoo - do we want any datafiles in our test suite? If not, is there another way to make them available for CI testing within the container?

@underwoo , @bensonr The Test33 reference dataset is six files of 132 KB each. Current size of the t directory immediately after a git clone is 887 MB.

@bensonr @underwoo @ceblanton
Concerning datafiles in our test suite, this perhaps should be theme that is part of the next release (after 2022.02), and I added I added existing issues ( #76 , #51) to the next project.

@ngs333 ngs333 merged commit a7eeb23 into NOAA-GFDL:master Dec 5, 2022
@ngs333 ngs333 self-assigned this Dec 5, 2022
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.

Implement do_cube_transform in make_hgrid
3 participants