379 continue integrating the ground water hydrology work into cable #411

rkutteh · 2024-09-30T04:29:38Z

This pull request contains ALL remaining ground water hydrology work of Mengyuan Mu.

PLEASE do not delete any of my in-code comments (tagged rk4417) for now. Once the ground water hydrology functionality is confirmed to reproduce the results obtained already outside of the repo, I will clean-up all of my comments in a single pull request. Deleting any of my comments now will just make it much more difficult for me to track down and fix any issues that might arise during this process.

📚 Documentation preview 📚: https://cable--411.org.readthedocs.build/en/411/

ccarouge

There are a lot of things in there. I let you digest but for a few things to start with:

revert the changes on soilparmnew. This isn't going to be introduced in this work. Happy to have it proposed as a standalone change.
I would like to move the addition of force_npatches_as also as a standalone issue and PR. And it will need associated documentation changes.

Also, since with this PR we will be able to evaluate the impact of the changes on the simulation without GW and with GW, I will want to remove all commented alternate versions before we merge. This can be done at the end, just putting in on the list of tasks.

src/offline/cable_checks.F90

ccarouge · 2024-10-11T03:09:41Z

src/offline/cable_driver.F90

+  nn = INDEX(inFile,'19')
+  READ(inFile(nn:nn+3),'(i4)') idummy
+  WRITE(inFile(nn:nn+3),'(i4.4)') ncciy
+  nn = INDEX(inFile,'19')
+  READ(inFile(nn:nn+3),'(i4)') idummy
+  WRITE(inFile(nn:nn+3),'(i4.4)') ncciy


why is it doing twice the same thing?

Is there any explanation anywhere of what this does?

I traced this back to the code from MMY (phase 1). The only explanation is what you see "MMY for Princeton input". It could be intentional or a simple duplication error. We can ping MMY about it of you like

src/offline/cable_driver.F90

src/util/cable_runtime_opts_mod.F90

ccarouge · 2024-10-29T05:10:00Z

src/util/cable_runtime_opts_mod.F90

+  !line below inserted to fix compilation error - rk4417 - phase2
+  INTEGER :: force_npatches_as=-1 


This new namelist option needs an entry in the documentation to explain what it is.

It does not seem to be related to the GW so I would rather introduce it separately than within the GW changes.

All I can say here is that I needed this line to get the code to compile at the time. Perhaps the present code will compile without it, I don't know. I suggest we leave this bit till the end to see whether it is still needed to get the code to compile.

src/science/soilsnow/cbl_stempv.F90

src/science/soilsnow/cbl_surfbv.F90

src/science/soilsnow/cbl_thermal.F90

ccarouge

There are a lot of things in there. I let you digest but for a few things to start with:

revert the changes on soilparmnew. This isn't going to be introduced in this work. Happy to have it proposed as a standalone change.
I would like to move the addition of force_npatches_as also as a standalone issue and PR. And it will need associated documentation changes.

Also, since with this PR we will be able to evaluate the impact of the changes on the simulation without GW and with GW, I will want to remove all commented alternate versions before we merge. This can be done at the end, just putting in on the list of tasks.

Reverted soilparmnew occurrence

revert occurrences of soilparmnew

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

ccarouge

@rkutteh A new wave of comments but they are for me to do stuff, nothing new for you yet.

ccarouge · 2025-01-16T03:45:01Z

src/offline/cable_mpimaster.F90

          ! Zero out lai where there is no vegetation acc. to veg. index
-          WHERE ( iveg%iveg(:) .GE. 14 ) iveg%vlai = 0.
-
+!          WHERE ( iveg%iveg(:) .GE. 14 ) iveg%vlai = 0.
+! line above replaced by below - rk4417 - phase2
+          WHERE ( veg%iveg(:) .GE. 14 ) veg%vlai = 0. ! MMY change from iveg%vlai to veg%vlai



This change looks correct but I'll need to double check (a note to myself...)

ccarouge · 2025-01-16T04:01:06Z

src/offline/cable_mpimaster.F90

+!             WHERE ( iveg%iveg(:) .GE. 14 ) iveg%vlai = 0.
+! line above replaced by below - rk4417 - phase2
+             WHERE ( veg%iveg(:) .GE. 14 ) veg%vlai = 0. ! MMY change from iveg%vlai to veg%vlai     
             ! Write time step's output to file if either: we're not spinning up


Same as above. Need to review the use of veg and iveg in the MPI implementation. (a note to myself)

ccarouge · 2025-01-16T05:00:24Z

src/offline/cable_parameters.F90

+    ! MMY start reading from gridinfo file  ! 2 lines inserted by rk4417 - phase2
+    ok = NF90_OPEN(filename%type, 0, ncid)
+    IF (ok /= NF90_NOERR) CALL nc_abort(ok, 'Error MMY finding gridinfo file.') ! MMY



Remove MMY from error message

Suggested change

! MMY start reading from gridinfo file ! 2 lines inserted by rk4417 - phase2

ok = NF90_OPEN(filename%type, 0, ncid)

IF (ok /= NF90_NOERR) CALL nc_abort(ok, 'Error MMY finding gridinfo file.') ! MMY

! MMY start reading from gridinfo file ! 2 lines inserted by rk4417 - phase2

ok = NF90_OPEN(filename%type, 0, ncid)

IF (ok /= NF90_NOERR) CALL nc_abort(ok, 'Error finding gridinfo file.') ! MMY

ccarouge · 2025-01-16T05:23:29Z

src/offline/cbl_model_driver_offline.F90

+
+    IF( cable_runtime%um ) THEN
+
+       IF( cable_runtime%um_implicit ) THEN
+          IF (cable_user%gw_model) THEN
+             CALL soil_snow_gw(dels, soil, ssnow, canopy, met, bal,veg)
+          ELSE
+             CALL soil_snow(dels, soil, ssnow, canopy, met, bal,veg)
+          ENDIF
+       ENDIF
+
+    ELSE
+       IF(cable_user%SOIL_STRUC=='default') THEN
+          IF (cable_user%gw_model) THEN
+             CALL soil_snow_gw(dels, soil, ssnow, canopy, met, bal,veg)
+          ELSE
+             CALL soil_snow(dels, soil, ssnow, canopy, met, bal,veg)
+          ENDIF
+       ELSEIF (cable_user%SOIL_STRUC=='sli') THEN
+
+          IF (cable_user%test_new_gw) &
+               CALL sli_hydrology(dels,ssnow,soil,veg,canopy)
+
+          CALL sli_main(ktau,dels,veg,soil,ssnow,met,canopy,air,rad,0)
+       ENDIF
+    ENDIF
+
+! I have inserted the above block from MMY code which was deleted from the trunk
+! soil_snow_gw and sli_hydrology are crucial in the GW module - rk4417 - phase2
+


This part of CABLE has change. This code is now only for offline with a different version of coupled, so the above block can be simplified as below:

Suggested change

IF( cable_runtime%um ) THEN

IF( cable_runtime%um_implicit ) THEN

IF (cable_user%gw_model) THEN

CALL soil_snow_gw(dels, soil, ssnow, canopy, met, bal,veg)

ELSE

CALL soil_snow(dels, soil, ssnow, canopy, met, bal,veg)

ENDIF

ENDIF

ELSE

IF(cable_user%SOIL_STRUC=='default') THEN

IF (cable_user%gw_model) THEN

CALL soil_snow_gw(dels, soil, ssnow, canopy, met, bal,veg)

ELSE

CALL soil_snow(dels, soil, ssnow, canopy, met, bal,veg)

ENDIF

ELSEIF (cable_user%SOIL_STRUC=='sli') THEN

IF (cable_user%test_new_gw) &

CALL sli_hydrology(dels,ssnow,soil,veg,canopy)

CALL sli_main(ktau,dels,veg,soil,ssnow,met,canopy,air,rad,0)

ENDIF

ENDIF

! I have inserted the above block from MMY code which was deleted from the trunk

! soil_snow_gw and sli_hydrology are crucial in the GW module - rk4417 - phase2

IF(cable_user%SOIL_STRUC=='default') THEN

IF (cable_user%gw_model) THEN

CALL soil_snow_gw(dels, soil, ssnow, canopy, met, bal,veg)

ELSE

CALL soil_snow(dels, soil, ssnow, canopy, met, bal,veg)

ENDIF

ELSEIF (cable_user%SOIL_STRUC=='sli') THEN

IF (cable_user%test_new_gw) &

CALL sli_hydrology(dels,ssnow,soil,veg,canopy)

CALL sli_main(ktau,dels,veg,soil,ssnow,met,canopy,air,rad,0)

ENDIF

! I have inserted the above block from MMY code which was deleted from the trunk

! soil_snow_gw and sli_hydrology are crucial in the GW module - rk4417 - phase2

ccarouge · 2025-01-16T05:41:14Z

src/science/canopy/cable_canopy.F90

+
+    IF (cable_user%gw_model) THEN       ! IF block inserted by rk4417 - phase2  
+       !   IF (call_number .eq. 1) call set_den_rat()
+       ssnow%wbliq(:,:) = ssnow%wb(:,:) - den_rat*ssnow%wbice(:,:)
+    ELSE
+       ssnow%wbliq(:,:) = ssnow%wb(:,:) - ssnow%wbice(:,:)
+    ENDIF
+


@har917 GW work introduces the calculation of wbliq at the start of cable_canopy.F90. Do you see anything here that would break the ACCESS coupling?
I also think we want the version with the ratio of liq/ice densities in all cases and not just for gw_model if we go with it. Can you confirm, please?

ccarouge · 2025-01-16T23:14:52Z

src/science/canopy/cbl_SurfaceWetness.F90

-       ssnow%wetfac = 0.5*(ssnow%wetfac + ssnow%owetfac)
+
+
+    CALL calc_srf_wet_fraction(ssnow,soil,met,veg)


This part of the code has been moved in the main branch. Will need to resolve conflicts before going further.

ccarouge · 2025-01-16T23:28:53Z

src/science/canopy/cbl_fwsoil.F90

       rwater = MAX(1.0e-9,                                                    &
-            SUM(veg%froot * MAX(1.0e-9,MIN(1.0, REAL((ssnow%wbliq -                 &
-            soil%swilt_vec)/(soil%sfc_vec-soil%swilt_vec)) )),2) )
+          SUM(veg%froot * MAX(1.0e-9, MIN( 1.0, &
+          MAX(0., (real(ssnow%wbliq) - real(soil%swilt_vec)) )&
+          / (real(soil%sfc_vec) - real(soil%swilt_vec)) &
+           )) , 2))


It's just uppercase.

ccarouge · 2025-01-16T23:30:07Z

src/science/canopy/cbl_fwsoil.F90

       rwater = MAX(1.0e-9,                                                    &
-            SUM(veg%froot * MAX(1.0e-9,MIN(1.0, REAL(ssnow%wb) -                   &
-            SPREAD(soil%swilt, 2, ms))),2) /(soil%sfc-soil%swilt))
-
-    ELSE
+          SUM(veg%froot * MAX(1.0e-9, MIN( 1.0, &
+          MAX(0., (real(ssnow%wb) - SPREAD(soil%swilt, 2, ms)) )&
+          / (SPREAD(soil%sfc, 2, ms) - SPREAD(soil%swilt, 2, ms)) &
+           )) , 2))


Re-reading it, it's the same equation as before but with an added MAX(0,x) on something that should always be positive and added SPREAD(). So we can add it, no need to test the effect.

ccarouge · 2025-01-16T23:58:22Z

src/science/soilsnow/cbl_smoisturev.F90

+                  / soil%zshh (k+1) + z2(:,k) * 0.5 * soil%zse( MAX( k-1,  &
+                  1 ) ) / soil%zshh (k) + z3(:,k+1) + z3(:,k) )



This seems it will impact all simulations, will need to test the impact on non-GW simulations then.

ccarouge · 2025-01-17T04:33:32Z

src/offline/cable_checks.F90


  TYPE(ranges_type), SAVE :: ranges

+  IF (cable_user%GW_MODEL) ranges%sucs = [30., 800.]   ! MMY the range [-0.8, -0.03] doesn't suit Mark Decker's version


Can't be done in the module definition part. This will require some more in-depth changes that will be done later.

ccarouge · 2025-03-14T04:12:15Z

A comparison of this branch with the previous version (at the end of stage 2 + fixes) show little statistical differences in results without GW, this suggests there is no obvious problem in the implementation in this branch for non-GW simulations.

ccarouge · 2025-03-17T03:46:41Z

modelevaluation.org analysis shows results stay very close to the original with the ground water on, so all tests passed. We can use this branch as the base for further work.

Next steps is to introduce changes that impact non-GW simulations separately (some of the issues might not need to be done before the GW work is adopted), then "rebase" (probably manually, too different for an automatic rebase). Then we should be good to get the rebased branch into the main.

ccarouge · 2025-04-24T02:45:50Z

This branch has become too out-of-date with the main branch. An updated branch and PR #580 have been created to bring this work up-to-date with the main branch. I am closing this pull request and will use the new one to continue the work. I will keep this original branch around for longer just in case, we need to check if anything is missing from the updated branch.

rkutteh added 6 commits September 6, 2024 21:08

Done with science/soilsnow

1b7e2e3

Done with science/canopy

d1b5994

Done with science/radiation

e9644f5

Done with science/gw_hydro

17650c5

Done with src/util

f73518c

Done with src/offline

10b7550

rkutteh added GWH priority:high High priority issues that should be included in the next release. labels Sep 30, 2024

rkutteh added this to the GW milestone Sep 30, 2024

rkutteh requested a review from ccarouge September 30, 2024 04:29

rkutteh self-assigned this Sep 30, 2024

rkutteh linked an issue Sep 30, 2024 that may be closed by this pull request

Continue integrating the ground water hydrology work into CABLE #379

Open

ccarouge reviewed Oct 29, 2024

View reviewed changes

rkutteh and others added 16 commits November 5, 2024 12:31

Update cable_driver.F90

ee4fb6b

Reverted soilparmnew occurrence

Update cable_driver.F90

8f354a0

revert occurrences of soilparmnew

Update src/offline/cable_initialise.F90

5288555

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_iovars.F90

c3b7559

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update cable_mpimaster.F90

17a3a16

Update src/offline/cable_mpimaster.F90

43af9ee

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_mpimaster.F90

8d73320

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_mpiworker.F90

d03d07f

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_mpiworker.F90

8b98ec2

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_mpiworker.F90

d9d19aa

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update cable_parameters.F90

b5f25af

Update src/offline/cable_parameters.F90

d6f92d7

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/offline/cable_parameters.F90

68f930e

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update cable_mpiworker.F90

2aeeb40

Update cable_mpiworker.F90

9955e81

Update src/offline/cable_output.F90

936acf8

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

rkutteh and others added 17 commits November 27, 2024 20:30

Update cable_parameters.F90

6d955ed

Update cable_parameters.F90

eb04b08

Update cable_parameters.F90

f3a262c

Update cable_canopy.F90

7bb3f32

Update cable_canopy.F90

1fd7ce4

Update cable_psm.F90

878fe12

Update cable_psm.F90

91a67ff

Update cbl_radiation.F90

844fc2a

Update src/science/soilsnow/cbl_snowCheck.F90

c51b50e

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update cable_canopy.F90

8014bf9

Update cable_canopy.F90

30e55b6

Update cable_canopy.F90

68ccad5

Update src/science/soilsnow/cbl_snowCheck.F90

0a79bcd

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update src/science/soilsnow/cbl_snowCheck.F90

68e4658

Co-authored-by: Claire Carouge <ccarouge@users.noreply.github.com>

Update cbl_snowCheck.F90

4ca6929

Update cbl_snowCheck.F90

4eb9a1a

Update cable_common.F90

34b1b77

rkutteh requested a review from ccarouge January 9, 2025 09:49

ccarouge reviewed Jan 17, 2025

View reviewed changes

ccarouge added 2 commits January 17, 2025 15:44

Add choice for 1D or 2D soil parameters in output

1fe7734

Fix for check_ranges() needs a rethink.

d3e39e1

ccarouge force-pushed the 379-continue-integrating-the-ground-water-hydrology-work-into-cable branch from eb6a56c to d3e39e1 Compare January 17, 2025 04:45

Princeton forcing does not provide snowfall.

a7a1c9a

ccarouge force-pushed the 379-continue-integrating-the-ground-water-hydrology-work-into-cable branch from 12b9505 to a7a1c9a Compare January 17, 2025 06:16

Remove superfluous code block to read LWdown

42bf0c5

ccarouge mentioned this pull request Apr 24, 2025

379 updated branch with all GW changes #580

Draft

3 tasks

ccarouge closed this Apr 24, 2025

		!line below inserted to fix compilation error - rk4417 - phase2
		INTEGER :: force_npatches_as=-1

		ssnow%wetfac = 0.5*(ssnow%wetfac + ssnow%owetfac)


		CALL calc_srf_wet_fraction(ssnow,soil,met,veg)

		/ soil%zshh (k+1) + z2(:,k) * 0.5 * soil%zse( MAX( k-1, &
		1 ) ) / soil%zshh (k) + z3(:,k+1) + z3(:,k) )


		TYPE(ranges_type), SAVE :: ranges

		IF (cable_user%GW_MODEL) ranges%sucs = [30., 800.] ! MMY the range [-0.8, -0.03] doesn't suit Mark Decker's version

379 continue integrating the ground water hydrology work into cable #411

379 continue integrating the ground water hydrology work into cable #411

Uh oh!

Conversation

rkutteh commented Sep 30, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccarouge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccarouge left a comment

Choose a reason for hiding this comment

Uh oh!

ccarouge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccarouge commented Mar 14, 2025

Uh oh!

ccarouge commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccarouge commented Apr 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rkutteh commented Sep 30, 2024 •

edited by github-actions bot

Loading

ccarouge commented Mar 17, 2025 •

edited

Loading