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

Skip static interpolation step if input grid isn't a unit sphere #1259

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

gdicker1
Copy link
Collaborator

@gdicker1 gdicker1 commented Jan 3, 2025

This PR adds a function to the init_atmosphere core to quickly check if a grid is likely a unit sphere and uses the function to throw an error if the static interpolation step is about to be repeated.

Most cases for the init_atmosphere model assume the input grid is a unit sphere and involve calculations to scale up to a new radius. Unlike the other cases, the case to generate real-data initial conditions is typically run a few times in sequence with different options. It's possible users could cause unintuitive issues by doubly scaling the input grid unless they take great care when editing namelist.init_atmosphere.

This PR protects against such errors. Now, running the init_atmosphere model to generate real-data initial conditions and attempting the static interpolation step with a non-unit sphere input grid will cause the model to abort with an error message.

This function uses the sum of cellArea for all owned cells to
approximate the surface area of the sphere. This function returns true
if the sum of cellArea is close to unit sphere surface area. Otherwise
this function returns false.

cellAreas are simple to sum, while direct checks of distance of cell
centers to the origin could be more expensive.
If static_interp is repeated, the mesh is scaled to unrealistic
dimensions. This can cause obvious science issues but can also cause
issues for running the init_atmosphere core. With unrealistic distances,
the process to assign soil categories may request too much memory and
cause the init_atmosphere core to fail.
@mgduda mgduda self-requested a review January 4, 2025 00:12
@@ -51,7 +51,6 @@ subroutine init_atm_setup_case(domain, stream_manager)
type (domain_type), intent(inout) :: domain
type (MPAS_streamManager_type), intent(inout) :: stream_manager


Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend not changing whitespace here.

!>
!> This routine will use the global sum of cell areas to
!> approximate the surface area of the sphere. If this surface area
!> is within a factor of 4*pii, the mesh is likely still a unit
Copy link
Contributor

Choose a reason for hiding this comment

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

The statement "within a factor of 4pii" could be misinterpreted to mean that the surface area is +/- 4pii of something.

Perhaps it would be better to reframe this description slightly to say that the routine returns true if it determines that the mesh is defined on a unit sphere -- which is the critical point -- and to then follow up with some notes that due to inexactness of arithmetic, the routine actually checks that the total sphere area is not larger than twice the area of a unit sphere.


surfArea = 0.0_RKIND
g_surfArea = 0.0_RKIND
tol = 2.0_RKIND
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a Fortran parameter (i.e., a const).

type (dm_info), intent(in) :: dminfo
type (mpas_pool_type), intent(inout) :: mesh

! Output vars
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest Return value rather than Output vars.

@gdicker1
Copy link
Collaborator Author

gdicker1 commented Jan 8, 2025

@mgduda, these fixup commits should address your comments. I'll rebase them if they're accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants