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

regrid_space fix #103

Merged
merged 1 commit into from
Jul 27, 2022
Merged

regrid_space fix #103

merged 1 commit into from
Jul 27, 2022

Conversation

LenkaNovak
Copy link
Collaborator

@LenkaNovak LenkaNovak commented Jul 27, 2022

PULL REQUEST

Purpose and Content

This fixes element order issues alter ClimaAtmos0.3.0 update.

Benefits and Risks

  • b: produces correct spaces, enables further development of snow and pipeline PRs
  • N/A

Linked Issues

Tests

  • land mask, initial ClimaAtmos Fields (unaltered by coupler) and solution ClimaAtmos Fields (altered by coupler) all produce the expected results. When testing, clear your environment - all temporary weightfiles should be deleted.

Screen Shot 2022-07-26 at 11 58 21 PM

Screen Shot 2022-07-26 at 11 57 59 PM

PR Checklist

  • This PR has a corresponding issue OR is linked to an SDI.
  • I have followed CliMA's codebase contribution and style guidelines OR N/A.
  • I have followed CliMA's documentation policy.
  • I have checked all issues and PRs and I certify that this PR does not duplicate an open PR.
  • I linted my code on my local machine prior to submission OR N/A.
  • Unit tests are included OR N/A.
  • Code used in an integration test OR N/A.
  • All tests ran successfully on my local machine OR N/A.
  • All classes, modules, and function contain docstrings OR N/A.
  • Documentation has been added/updated OR N/A.

@LenkaNovak LenkaNovak requested a review from kmdeck July 27, 2022 07:01
@LenkaNovak LenkaNovak force-pushed the regrid_space branch 2 times, most recently from 6e5f989 to f08959c Compare July 27, 2022 13:59

land_mask = ClimaCore.Fields.zeros(boundary_space)
land_mask_t = ClimaCore.Fields.zeros(boundary_space_t)
Copy link
Member

Choose a reason for hiding this comment

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

Im curious about the _t subscript here :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It stands for test. 😇 I know this file only a temporary way of testing, but it was overwriting the variables from the driver, which made debugging a bit harder. :-P

@kmdeck
Copy link
Member

kmdeck commented Jul 27, 2022

Looks great Lenka. as far as I can tell this removes the regrid space and replaces it with boundary_space in each case. If this fixes the plotting issue it is good to go!

One minor fix - one of the regrid .g files slipped into the PR.

@LenkaNovak LenkaNovak marked this pull request as ready for review July 27, 2022 15:59
arg fix

arg fix

rm regrid dir

rm .g file
@LenkaNovak
Copy link
Collaborator Author

Looks great Lenka. as far as I can tell this removes the regrid space and replaces it with boundary_space in each case. If this fixes the plotting issue it is good to go!

One minor fix - one of the regrid .g files slipped into the PR.
Ooh! Nice catch! 😃

@LenkaNovak
Copy link
Collaborator Author

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 27, 2022

@bors bors bot merged commit cd4c6f4 into main Jul 27, 2022
@bors bors bot deleted the regrid_space branch July 27, 2022 18:59
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.

Fix remapping of NetCDF boundary condition files
2 participants