Feature/borescan merge sm peat #524

biljanaorescanin · 2022-01-25T19:06:18Z

Before Sarith left the land group he was working on several PR's.
This PR is reproducing results he has shown for the update related to peatlands.

Related issue and PR:
PR: #417
Issue: #469

I'm able to get zero diff comparison for both history and restart files with respect to my "offline" run when:

In "lfs" collection I remove SWnet variable ( we don't have SWnet in our collection anymore)
For "lnd" collection I have to run experiment with no nbits.

@gmao-rreichle @rdkoster

UPDATED 9-Feb-2022 by @gmao-rreichle:
The above 0-diff result is vs. Sarith's original PEATCLSM simulation:
/discover/nobackup/borescan/par/NL5_reproduce_NLv4p_M01_M07_nbits/

UPDATED 11 Feb 2022 by @gmao-rreichle:

Summary of Changes:

Added revised hydrology for peatlands (PEATCLSM): New PEATCLSM hydrology is used when the model is run with "NLv5" boundary conditions, which identify peat tiles through hybridization of PEATMAP and HWSD ancillary data and assign a porosity of 0.93. PEATCLSM hydrology kicks in for tiles with porosity >=0.9.
New Catchment export variables: WATERTABLED, FSWCHANGE
New Catchment diagnostic functions: catch_calc_zbar(), catch_calc_watertabled()
Cleanup

Test Results (at final commit 90f7439):

0-diff vs. AGCM develop
0-diff vs. GEOSldas develop for all Intel and GNU tests (as expected, tests with “aggressive” builds have roundoff differences)
0-diff vs. Sarith's original PEATCLSM experiment after reversing the following fix that was needed to get 0-diff vs. GEOSldas develop: https://github.com/GEOS-ESM/GEOSgcm_GridComp/compare/f10f98e..59e3121
Successfully reproduces bcs (verified for M36/NLv3 and M36/NLv5)

Merge Info: The present PR already includes and should be merged after #536.

Still To Do: See issue #539 for future PEATCLSM work.

…CLSM : - enable use of vectorized catch_calc_zbar() to compute WATERTABLED export - remove unnecessary -1.e-20 in rzdrain() - simplify ramp function implementation for surface runoff as fn of ar1

gmao-rreichle · 2022-02-08T23:45:14Z

Added non-zero-diff PEATCLSM cleanup: commit 9dcb682.
These changes are non-zero-diff w.r.t. Sarith's original experiment (see comment above).

…_process_hres_data.F90

…rge_sm_peat

…an_merge_sm_peat: Manually resolved conflicts: [..]/GEOSland_GridComp/GEOScatchCN_GridComp/GEOScatchCNCLM40_GridComp/GEOS_CatchCNCLM40GridComp.F90 [..]/GEOSland_GridComp/GEOScatchCN_GridComp/GEOScatchCNCLM45_GridComp/GEOS_CatchCNCLM45GridComp.F90 [..]/GEOSland_GridComp/GEOScatchCN_GridComp/Shared/catchmentCN.F90 [..]/GEOSland_GridComp/GEOScatch_GridComp/catchment.F90 [..]/GEOSland_GridComp/Shared/lsm_routines.F90

- in Scale_Catch[CN].F90, avoid resetting catdef etc for peat tiles that are also peat in source (old) restart - renamed "process_peat" to "use_PEATMAP" for clarity

sanAkel · 2022-02-13T18:26:23Z

@biljanaorescanin, @gmao-rreichle,
I will review on behalf of the @GEOS-ESM/ocean-team. Please help me gather a few things:

Once I build this branch: feature/borescan_merge_sm_peat
Re-generate the BCs using make_bcs for my favorite ocean resolution(s), but using NLv3
I expect the SAME or almost same answer as I have from the develop version of GEOSgcm_GridComp ⬅️ is this correct?

If ⬆️ checks out, that would be fine with me.

I will try to get back to you by end of this week. 🙏

gmao-rreichle · 2022-02-13T18:55:44Z

@sanAkel, @mathomp4, @yvikhlya, @zhaobin74, @lcandre2, @sdrabenh:

Many thanks, @sanAkel, for offering to review on behalf of the @GEOS-ESM/ocean-team, which made me realize that we must have touched files (probably GEOSsurface_GridComp/Utils to do with restarts and bcs) that require approval from the @GEOS-ESM/ocean-team and @GEOS-ESM/landice-team. This is normally a good thing, but in this particular case it throws a major wrench into the works. We need this PR to be merged immediately, before the big GOCART-2 changes. I've been in touch with @sdrabenh about this.

The PR introduces new peat physics that are important and can't be delayed for the next SMAP L4 product release. My very strong preference would be for the ocean and landice teams (or @mathomp4 for both) to take my word for this and approve the PR so that @sdrabenh can proceed.

The changes in this PR, incl. those in GEOSsurface_GridComp/Utils, only impact Catchment and do not impact ocean or landice. From the perspective of ocean and landice, the PR is trivially 0-diff. Also, we verified that the PR is 0-diff, which @sdrabenh is also verifying on his end. Changes in bcs are only cosmetic and limited to land (improved documentation and improved consistency of mask inputs for EASE-grid land bcs, which aren't used by ocean or landice).

Sorry I didn't catch the ocean and landice connections earlier and failed to give everyone a heads-up. And I now also realize that the ocean and landice teams were probably bombarded with a ridiculous number of GitHub emails from this PR. Sorry!!!

sanAkel

Changes in GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/GEOS_SurfaceGridComp.F90
look okay

sanAkel · 2022-02-13T19:07:34Z

@gmao-rreichle, NP! Following is comforting (I still feel the burn from NLv3 default! 😞 )

The changes in this PR, incl. those in GEOSsurface_GridComp/Utils, only impact Catchment and do not impact ocean or landice. From the perspective of ocean and landice, the PR is trivially 0-diff. Also, we verified that the PR is 0-diff, which @sdrabenh is also verifying on his end. Changes in bcs are only cosmetic and limited to land (improved documentation and improved consistency of mask inputs for EASE-grid land bcs, which aren't used by ocean or landice).

We (@GEOS-ESM/ocean-team) co-own GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/GEOS_SurfaceGridComp.F90

GEOSgcm_GridComp/.github/CODEOWNERS

Line 36 in 7bae5aa

    
           /GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/GEOS_SurfaceGridComp.F90 @GEOS-ESM/land-team @GEOS-ESM/landice-team @GEOS-ESM/ocean-team

(though I am not sure if these reviews are strictly being enforced!) and indeed your changes for it look okay, the only thing I would like to note for now is probably to start paying attention to the added expense of LocStreamTransform ... Otherwise, all seems okay!

Thanks again!

gmao-rreichle · 2022-02-13T19:22:36Z

Thanks, @sanAkel, for pointing out the details of ownership. (Among the changed files, there's also overlap with landice for SurfParams.F90, but there are only whitespace differences.)
Re. the LocStreamTransform: I thought this only kicks in when the new (peat-related) diagnostics are written out in HISTORY, but I might be wrong. In any case, there are only 2 new diagnostics, which are important going forward, while there are many more (pre-existing) diagnostics associated with dirty snow that are useless at this time. If there is a cost to keeping these around, a (separate) cleanup of the dirty snow elements would provide a more fruitful approach to realize savings.

sanAkel · 2022-02-13T19:28:55Z

Thanks, @sanAkel, for pointing out the details of ownership. (Among the changed files, there's also overlap with landice for SurfParams.F90, but there are only whitespace differences.) Re. the LocStreamTransform: I thought this only kicks in when the new (peat-related) diagnostics are written out in HISTORY, but I might be wrong. In any case, there are only 2 new diagnostics, which are important going forward, while there are many more (pre-existing) diagnostics associated with dirty snow that are useless at this time. If there is a cost to keeping these around, a (separate) cleanup of the dirty snow elements would provide a more fruitful approach to realize savings.

@gmao-rreichle if these extra ones are for HISTORY, then, of course, there won't be any addition if these are not present in the HISTORY. (However, in the past, some of us - incl. myself! added our developments and corresponding additions to HISTORY, which ultimately got tagged along and included in stock versions, and sadly enough nobody cared else cared for those fields! With this github way, hopefully we will use such versions of HISTORY in our local branches, instead of committing to the the develop version of the App.)

gmao-rreichle · 2022-02-14T14:49:41Z

Many thanks, @sdrabenh, for moving ahead with the peat PR. Much appreciated. It beats me, though, why this was possible without approval from the @GEOS-ESM/landice-team. @mathomp4: Would this be something to take a closer look at?

mathomp4 · 2022-02-14T16:04:13Z

Many thanks, @sdrabenh, for moving ahead with the peat PR. Much appreciated. It beats me, though, why this was possible without approval from the @GEOS-ESM/landice-team. @mathomp4: Would this be something to take a closer look at?

Huh. As far as I can see from the settings, it should have. Very odd. I'll have to research this and see.

lcandre2 · 2022-02-14T16:15:03Z

Many thanks, @sdrabenh, for moving ahead with the peat PR. Much appreciated. It beats me, though, why this was possible without approval from the @GEOS-ESM/landice-team. @mathomp4: Would this be something to take a closer look at?

Huh. As far as I can see from the settings, it should have. Very odd. I'll have to research this and see.

I did come to look at/approve this this morning and didn't see any approval needed.

biljanaorescanin added 30 commits December 20, 2021 11:41

Merging SM old code with current develop

6cdcc7d

define FSW_CHANGE

f319d72

add peat source code "POROS_HighLat"

d939ed6

add vars to srunoff

79d3157

define type

be25bcc

adding rzexc type

1a82678

define var types

58170b0

adding vars to sobroutine

66b4e67

more vars need type

907a493

edit

d345689

minor edit

51ca254

enable NLv4p option in make_bcs

4ca7479

order of arguments

57bcb38

clean up

9bf1aa7

bf1 and bf2 to call CATCH_CALC_SOIL_MOIST

43fcbe1

more cleanup

d160893

bf1 and bf2 were missing here

096bd96

edits

e9fb2b9

typo

d85f25e

missing porosity at some calls

8560022

poros in subtoutine calls

2bc3180

wrong code placement

14f253e

missing decelaratio

3cf5a62

more edits

c78bb0c

fix type

f8651e1

forgot to save change

614c6bb

fixing alignment

e32e95f

bug fix

af94537

Merge branch 'develop' into feature/borescan_merge_sm_peat

f648049

align list

d2daaad

additional cleanup: assorted roundoff (non-zero-diff) changes in PEAT…

9dcb682

…CLSM : - enable use of vectorized catch_calc_zbar() to compute WATERTABLED export - remove unnecessary -1.e-20 in rzdrain() - simplify ramp function implementation for surface runoff as fn of ar1

gmao-rreichle added 4 commits February 9, 2022 20:32

commented out presumably obsolete subroutine process_peatmap() in mod…

19b87d7

…_process_hres_data.F90

Merge branch 'feature/borescan_fix_make_bcs' into feature/borescan_me…

03ab65f

…rge_sm_peat

fixes in restart processing; cleanup:

90f7439

- in Scale_Catch[CN].F90, avoid resetting catdef etc for peat tiles that are also peat in source (old) restart - renamed "process_peat" to "use_PEATMAP" for clarity

gmao-rreichle mentioned this pull request Feb 11, 2022

PEATCLSM outstanding issues #539

Open

6 tasks

gmao-rreichle removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Feb 11, 2022

gmao-rreichle marked this pull request as ready for review February 11, 2022 21:43

gmao-rreichle requested review from a team as code owners February 11, 2022 21:43

gmao-rreichle approved these changes Feb 11, 2022

View reviewed changes

gmao-rreichle assigned sdrabenh Feb 11, 2022

sanAkel approved these changes Feb 13, 2022

View reviewed changes

sdrabenh approved these changes Feb 13, 2022

View reviewed changes

Merge branch 'develop' into feature/borescan_merge_sm_peat

24635de

sdrabenh merged commit db83d28 into develop Feb 13, 2022

sdrabenh deleted the feature/borescan_merge_sm_peat branch February 13, 2022 22:52

gmao-rreichle mentioned this pull request Feb 14, 2022

Land Group Peat and Irrigation PR's #469

Open

gmao-rreichle mentioned this pull request Jun 3, 2022

bug fix: PEATCLSM water table depth #593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/borescan merge sm peat #524

Feature/borescan merge sm peat #524

biljanaorescanin commented Jan 25, 2022 •

edited by gmao-rreichle

Loading

gmao-rreichle commented Feb 8, 2022

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 13, 2022

sanAkel left a comment

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 13, 2022

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 14, 2022

mathomp4 commented Feb 14, 2022

lcandre2 commented Feb 14, 2022

Feature/borescan merge sm peat #524

Feature/borescan merge sm peat #524

Conversation

biljanaorescanin commented Jan 25, 2022 • edited by gmao-rreichle Loading

gmao-rreichle commented Feb 8, 2022

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 13, 2022

sanAkel left a comment

Choose a reason for hiding this comment

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 13, 2022

sanAkel commented Feb 13, 2022

gmao-rreichle commented Feb 14, 2022

mathomp4 commented Feb 14, 2022

lcandre2 commented Feb 14, 2022

biljanaorescanin commented Jan 25, 2022 •

edited by gmao-rreichle

Loading