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

Make usrf condition in is_in_active_grid consistent with other checks #40

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

billsacks
Copy link
Member

As per suggestions from Bill Lipscomb in
#39, I am treating usrf == 0 as
land rather than ocean. This makes the conditional here consistent with
similar checks elsewhere in CISM.

As per suggestions from Bill Lipscomb in
ESCOMP#39, I am treating usrf == 0 as
land rather than ocean. This makes the conditional here consistent with
similar checks elsewhere in CISM.
@billsacks
Copy link
Member Author

I'm going ahead and merging this since it just does what we discussed in #39, and merging it will let me run testing in the context of CISM-wrapper. @whlipscomb feel free to comment after the fact if you want.

@billsacks billsacks merged commit ed80880 into ESCOMP:main Aug 31, 2021
@billsacks billsacks deleted the usrf0_consistency branch August 31, 2021 23:20
@billsacks
Copy link
Member Author

I meant to mention: I checked the standard 4-km Greenland input file, and it has 70 grid cells with topg exactly equal to 0, and 50 grid cells with (topg + thk) (i.e., usrf) exactly equal to 0. So I expect small answer changes from this change. (I had been hoping that we could avoid answer changes in a Greenland case, but I don't feel that it's a big deal that it will change answers: I'll just want to look more closely at the differences to make sure there aren't any significant unforeseen consequences of this change.)

@whlipscomb
Copy link
Contributor

@billsacks – I agree that small answer changes for Greenland aren't a big deal. I would hope that most or all of the points with topg = usrf = 0.0 are in the interior, surrounded by cells with usrf > 0. In that case, labeling them as land will reduce the chance of having weird logic errors. But if there are such points out in the ocean, we might have to give them a closer look.

@billsacks
Copy link
Member Author

Yes, they are either in the interior or on the coast: I don't see any that are out in the ocean.

Here is a map with a range and color scale that makes these points with topg == 0 stand out: see the green pixels in this map:

image

@whlipscomb
Copy link
Contributor

@billsacks – Thanks for the image. I think that reclassifying these points as land would be a good thing, or at least not a bad thing.

billsacks added a commit that referenced this pull request Sep 1, 2021
Revert PR #40

This didn't work: it resulted in the ice mask being 1 everywhere.

See #39 (comment)

We'll fix this later (see #41)
@billsacks
Copy link
Member Author

As discussed in #39 and #41 this didn't work right. I have reverted it (#42 ) and we'll revisit #41 later.

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.

2 participants