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

Move mksurfdata_map rounding of special landunits to before wetland fill #1119

Merged
merged 2 commits into from
Aug 29, 2020

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Aug 21, 2020

Description of changes

I think this may be important to avoid errors and/or wrong behavior in
some edge cases where we have < 0.5% cover of glacier and/or lake, and
other cases.

See #1118 for detailed thoughts

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #):
Resolves #1118

Are answers expected to change (and if so in what way)? Possible answer changes, but this should be rare

Any User Interface Changes (namelist or namelist defaults changes)? N

Testing performed, if any:

I created a subset of our standard surface datasets with make -f Makefile.data global-present before and after this change. The three surface datasets created in this way (f09, f19 and f10) were all bit-for-bit. There could be answer changes in rare cases, but this demonstrates that I haven't majorly broken anything with this change.

  • Update (2020-08-27): as noted below, I also compared f45.

I think this may be important to avoid errors and/or wrong behavior in
some edge cases where we have < 0.5% cover of glacier and/or lake, and
other cases.

See ESCOMP#1118 for detailed thoughts

Resolves ESCOMP#1118
@billsacks billsacks added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Aug 21, 2020
@billsacks billsacks requested a review from ekluzek August 21, 2020 22:07
@billsacks billsacks assigned ekluzek and unassigned ekluzek Aug 21, 2020
@billsacks
Copy link
Member Author

@ekluzek it seems like it would be good to use these diffs when you recreate all surface datasets. Would you like me to bring this in on its own? If so, are you comfortable with my testing including (a) the comparison of the three surface datasets f09, f19 and f10, plus (b) standard tools testing? (Or would you rather just fold this into your tag where you update surface datasets?)

@billsacks
Copy link
Member Author

At @ekluzek 's request, I also compared 4x5 surface datasets with and without this change and they, too, are identical (from ./mksurfdata.pl -no-crop -vic -glc_nec 10 -y 2000 -res 4x5)

@billsacks
Copy link
Member Author

Tools tests all pass, with comparison against ctsm1.0.dev110

@billsacks billsacks merged commit 2649b38 into ESCOMP:master Aug 29, 2020
@billsacks billsacks deleted the mksurfdata_map_truncation branch August 29, 2020 01:55
samsrabin pushed a commit to samsrabin/CTSM that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
Status: Done (non release/external)
Development

Successfully merging this pull request may close these issues.

mksurfdata_map rounding/truncation of special landunits should happen before wetland filling
2 participants