Bare soil evaporation separated over irrigated and non-irrigated soil #139 - #150
Bare soil evaporation separated over irrigated and non-irrigated soil #139#150Nic Gedney (nicgedney) wants to merge 25 commits into
Conversation
github-actions
Bot
added
the
cla-required
Nic Gedney (nicgedney)
changed the title
Nic Gedney (nicgedney)
assigned
Nic Gedney (nicgedney) and
hkatty and unassigned
hkatty and
Nic Gedney (nicgedney)
John Edwards (JMEdwardsXtr)
left a comment
John Edwards (JMEdwardsXtr)
left a comment
There was a problem hiding this comment.
As yet, I only partially understand the science here, but I have made such comments as I can. I would like the logic to be more apparent. I will be happy to look again once these comments have been addressed.
I'm not sure exactly what reviewing role I should be performing here. The PR says that hkatty will do the science review, but approval is also required from the owners of the relevant JULES modules, which approval I can give for the surface module.
| No effect. | ||
|
|
||
| This must be set to FALSE if :nml:mem:`JULES_IRRIG::irrig_option` = 0. | ||
| This must be set to FALSE if :nml:mem:`JULES_IRRIG::irrig_option` = 2. |
There was a problem hiding this comment.
I'm probably going to seem foolish here, but I'm having trouble working out what is going on. My interpretation is that irrig_opt=0 means no irrigation, irrig_opt=1 will mean the current scheme that is currently represented by l_irrig_dmd=.T., and that irrig_opt=2 will be the new scheme. So this says that the option only works with the current scheme, but then I am confused by L2649 in rose-meta/jules-standalone/HEAD/rose-meta.conf where the option is triggered if l_irrig_dmd is .F.
As a cautionary query here, if it's meant to work with l_irrig_dmd=.T., can I check that no modification of src/science/soil/hydrol_jls_mod.F90 is needed because the extraction is already treated properly?
There was a problem hiding this comment.
Yes it's been complicated as Heather has put in a new version of the irrigation code (and so has added irrig_option=2), so at some point l_irrig_dmd will be replaced with irrig_option=1. However this new option only works for l_irrig_dmd=T (soon to be irrig_option=1). As previously hydrol_jls takes the (irrig and grid-box mean) extraction terms and soil moisture and applies the extraction and flows and applies accordingly to the irrigation/non-irrig soil columns. Only the grid box mean water table depth and lateral flows are still calculated based on the grid box mean moisture terms.
| * esoil_surft(l,n) | ||
|
|
||
| IF ( l_soil_evap_irrig_expl ) THEN | ||
| wt_ext_nir_surft(l,m,n) = wt_ext_surft(l,m,n) |
There was a problem hiding this comment.
You could use an IF...ELSE...END IF construct here, but I'm not sure whether that's faster than what you have on current machines or not.
There was a problem hiding this comment.
I'm leaving that to the UM systems team
| END DO | ||
| !$OMP END DO NOWAIT | ||
| END IF | ||
| IF (l_soil_evap_irrig_expl) THEN |
There was a problem hiding this comment.
I haven't properly understood the relationship between this variable and l_irrig_dmd.. In sf_evap_jls we had a general block of code and another under l_irrig_dmd. Can I just check that everything is consistent here?
There was a problem hiding this comment.
l_soil_evap_irrig_expl can only be try is l_irrig_dmd is true. If the former logical is false - ext_nir can be calculated from wt_ext_irr and wt_ext. If logical is true then wt_ext_nir needs to be calculated explicitly and then the grid box mean wt_ext is calculated as a weighted ave of the two columns.
In both calses wt_ext_irr has to be calculated explicitly. Does that make sense?
| q1_land(l) = qw_1(i,j) | ||
| cosz_gb(l) = cos_zenith_angle(i,j) | ||
| fsoil_tot(l) = frac(l,soil) | ||
| IF ( l_soil_evap_irrig_expl ) THEN |
There was a problem hiding this comment.
This may be a matter of algorithmic convenience, but why are we irrigating the bare-soil tile?
There was a problem hiding this comment.
It would happen by default, the extent of which will depends on the irrigation technique/conditions. Crops dont overlap perfectly, vegetation grows etc.
(I've also just noticed that in the code gsoil_under_canopy has been referred to a bare soil conductance under canopy for as long back as I can see).
|
OFFICIAL
Thanks Nic,
If it's just the principle of modifying the surface module that you're interested in, I'm happy to approve the code, but I don't seem to be able to do so. The surface module just has "* []" next to it, not a checkbox. (I did have an ulterior motive in doing the review in that I wanted to see the new process from the other side before I have to do the same!)
Do you want me to mention that this is going in during next week's module update?
Regards,
John
OFFICIAL
________________________________
From: Nic Gedney ***@***.***>
Sent: 01 September 2026 11:46
To: MetOffice/jules ***@***.***>
Cc: John M.Edwards ***@***.***>; Review requested ***@***.***>
Subject: Re: [MetOffice/jules] Bare soil evaporation separated over irrigated and non-irrigated soil #139 (PR #150)
@nicgedney commented on this pull request.
________________________________
In doc/source/namelists/jules_irrig.nml.rst<#150 (comment)>:
@@ -148,6 +148,24 @@ This namelist specifies the different options available for setting up the irrig
:nml:mem:`nstep_irrig` = NINT(frequency of irrigation update (in sec)) / :nml:mem:`JULES_TIME::timestep_len`
+.. nml:member:: l_soil_evap_irrig_expl
+
+ :type: logical
+ :default: F
+
+ Switch controlling whether the bare soil evaporation from the irrigated and non-irrigated part of the grid-box (or soil tile) is controlled by the mean soil moisture or the separate irrigated and non-irrigated soil moisture columns.
Yes, John I was just asking for module leader approval, but your input is most welcome!
—
Reply to this email directly, view it on GitHub<#150?email_source=notifications&email_token=ATL2IIFUYRIZT6WRTIYNVWT5M2SJ7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBXG4YDIMJUGMZKM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#discussion_r3903309851>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATL2IIHEW544OQ5BG5FBAHT5M2SJ7AVCNFSNUABGKJSXA33TNF2G64TZHMYTAOBWGUZTENJQGI5US43TOVSTWNJSGU4DEOJQHEZDLILWAI>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/ATL2IIEVAQGZEQJBAZU3YEL5M2SJ7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBXG4YDIMJUGMZKM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/ATL2IIF74VEUWRCMIDOZDGD5M2SJ7A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBXG4YDIMJUGMZKM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because your review was requested.Message ID: ***@***.***>
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new irrigation option to compute bare-soil evaporation explicitly over irrigated vs non‑irrigated fractions (using their separate soil-moisture columns) instead of using the grid-box mean soil moisture. It adds a new namelist switch (l_soil_evap_irrig_expl) and threads the required fraction-specific conductances/extraction terms through the surface physiology/evaporation/resistance code paths, with Rose metadata, upgrade macro, and documentation updates.
Changes:
- Add
l_soil_evap_irrig_explswitch (namelist + validation) to enable explicit irrigated/non‑irrigated bare-soil evaporation. - Plumb irrigated/non‑irrigated conductances and extraction fractions through
physiol,soil_evap,sf_resist, and call sites. - Add Rose metadata + upgrade macro entry and update namelist documentation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/science/surface/soil_evap_jls.F90 | Adds explicit irrigated/non‑irrigated soil evaporation weighting using frac_irr and new nir arrays. |
| src/science/surface/sf_resist_jls.F90 | Updates surface resistance calculation to optionally blend irrigated/non‑irrigated canopy conductances. |
| src/science/surface/sf_flux_mod.F90 | Argument formatting update (no functional change apparent in diff hunk). |
| src/science/surface/sf_evap_jls.F90 | Adds non‑irrigated extraction bookkeeping for explicit irrigated vs non‑irrigated evaporation. |
| src/science/surface/physiol_jls_mod.F90 | Computes/propagates nir conductances and soil-moisture terms; updates soil evaporation call signature. |
| src/science/surface/jules_ssi_sf_explicit_jls.F90 | Updates fcdch call sites to match new sf_resist/conductance arguments. |
| src/science/surface/jules_land_sf_explicit_jls.F90 | Threads new conductance/fraction arguments through land explicit surface coupling. |
| src/science/surface/fcdch.F90 | Extends sf_resist calls/signature to pass irrigated conductance and irrigation fraction. |
| src/control/shared/jules_irrig_mod.F90 | Adds l_soil_evap_irrig_expl switch, namelist entry, and validation. |
| rose-meta/jules-standalone/versions.py | Adds a Rose upgrade macro to populate the new namelist member. |
| rose-meta/jules-standalone/HEAD/rose-meta.conf | Adds Rose metadata entry for the new namelist member. |
| doc/source/namelists/jules_irrig.nml.rst | Documents the new l_soil_evap_irrig_expl namelist option. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Suppressed comments (3)
src/science/surface/physiol_jls_mod.F90:843
- This loop nests an inner
IF ( l_irrig_dmd )inside an outerIF ( l_irrig_dmd ), and repeatssthu_nir_soilt = sthu_soilttwice. This is redundant and makes the logic harder to follow; it can be simplified to a single conditional on frac_irr_soilt.
sthu_nir_soilt(l,m,k) = sthu_soilt(l,m,k)
IF ( l_irrig_dmd ) THEN
sthu_nir_soilt(l,m,k) = sthu_soilt(l,m,k)
IF ( frac_irr_soilt(l,m) < 1.0 ) THEN
sthu_nir_soilt(l,m,k) = &
rose-meta/jules-standalone/versions.py:51
- This upgrade macro still has template placeholders in the class name and docstring (vnYY_txxxx / "by Author"). Please rename the class to match the tag (and remove placeholder text) to be consistent with the existing rose upgrade macros.
class vnYY_txxxx(MacroUpgrade):
"""Upgrade macro from JULES by Author"""
src/science/surface/jules_land_sf_explicit_jls.F90:1256
- There is a commented-out argument ("not needed?") in the call argument list. Please remove it to avoid ambiguity about the actual calling interface.
gs_irr_surft, smc_irr_soilt, wt_ext_irr_surft, gc_irr_surft, &
gs_nir_surft, &
!! not needed? gc_nir_surft, &
| !$OMP PARALLEL IF(l_do_omp) DEFAULT(NONE) PRIVATE(l,m,n) SHARED(dzsoil, & | ||
| !$OMP fsoil_tot, land_pts, smc_irr_soilt, sthu_irr_soilt,nsoilt, & | ||
| !$OMP smvcst_soilt, gs_irr_surft,gc_irr_surft, nsurft,l_do_omp) | ||
| !$OMP fsoil_tot, land_pts, smc_irr_soilt, sthu_irr_soilt, nsoilt, & | ||
| !$OMP smc_nir_soilt, smc_soilt, sthu_nir_soilt, & | ||
| !$OMP smvcst_soilt, gs_irr_surft, gc_irr_surft, nsurft, l_do_omp, & | ||
| !$OMP fsoil_irr_tot, fsoil_nir_tot, frac_irr_soilt, & |
| !$OMP END DO NOWAIT | ||
| END IF | ||
| IF (l_soil_evap_irrig_expl) THEN | ||
| !$OMP DO SCHEDULE(STATIC) |
github-actions
Bot
added
cla-signed
PR Summary
<-- Bare soil evaporation separated over irrigated and non-irrigated soil #139 -->
Sci/Tech Reviewer: <-- hkatty -->
Code Reviewer:
<-- #139 -->
<-- this allows the irrigated and non-irrigated bare soil evaporation terms to be calculated from the irrigated and non-irrigated soil moisture columns (l_soil_evap_irrig_expl=True) rather than the grid-box mean soil moisture (l_soil_evap_irrig_expl=False) as previously -->
<-- List any linked PRs here
None
-->
<-- List any blocking PRs or issues to be closed here
None
-->
Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
rose-meta/jules-sharedthen have you supplied a linked UM and LFRic Apps PR?Testing
<-- Other testing performed:
rose-stem test also applied to the test branch test_separate_irrigated_bare_soil_evap
individual point comparison with l_soil_evap_irrig_expl=T compared with stable
-->
trac.log
<--
Development branch:
Test Suite Results -
Test Suite Results - jules - vn8.2_separate_irrigated_bare_soil_evap/run3
Suite Information
Task Information
✅ succeeded tasks - 676
Test branch:
Test Suite Results - jules - vn8.2_test_separate_irrigated_bare_soil_evap/run3
Suite Information
Task Information
✅ succeeded tasks - 676
-->
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
Approvals
Please request all relevant approvals. See the CodeOwners.txt file for section owners.
Technical
Scientific
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review