fix l bgc heat - #153
fix l bgc heat#153Eleanor Burke (eleanorgb) wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the biogeochemical soil heating option (l_bgc_heat) so it can be used in UM-JULES configurations and with the 1-layer soil carbon setup, primarily by aligning the respiration/heating logic with the soil-carbon layer dimension (dim_cslayer) and removing prior “standalone-only” gating.
Changes:
- Update
soil_htcbiogenic heating to usedim_cslayer(carbon-layer dimension) rather thannshyd(hydrology/soil moisture levels). - Remove UM-side unavailability checks and standalone rose-meta constraints/warnings that prevented enabling
l_bgc_heat. - Add a new entry to
CONTRIBUTORS.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/science/soil/soil_htc_jls_mod.F90 |
Switch biogenic heating loops/array shape to use dim_cslayer and add required module import/OpenMP shared var. |
src/control/um/check_unavailable_options_mod.F90 |
Remove the UM runtime message that previously rejected l_bgc_heat. |
src/control/shared/jules_soil_biogeochem_mod.F90 |
Relax configuration checks that previously required l_q10/l_layeredc for l_bgc_heat. |
rose-meta/jules-standalone/HEAD/rose-meta.conf |
Remove standalone metadata restriction and warning tied to UM and l_q10 when using l_bgc_heat. |
CONTRIBUTORS.md |
Add contributor entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Maggie (maggiehendry)
left a comment
There was a problem hiding this comment.
Changes look fine to me. I'd just like to check the logic. Thanks.
| 'you must use l_q10=.true. and l_bgc_heat=.true.' ) | ||
| END IF | ||
|
|
||
| ! Check if l_layeredC=T to use l_bgc_heat |
There was a problem hiding this comment.
Am I right in thinking that these changes allow l_bgc_heat to be used either with l_layeredC=.true. or l_layeredC=.false.
There was a problem hiding this comment.
This is correct
Erica Neininger (ericaneininger)
left a comment
There was a problem hiding this comment.
These changes look fine to me.
Please could you update to the head of main, and ensure that the rose-meta changes are up to date with the move of all UM metadata to this repository (see PR #155) - and therefore close the related UM PR as I believe it is no longer required - those changed should instead be made on this branch.
I think Ive updated to head of the main? |
It looks like you have done a merge, but something is not quite right - what you have done is rolled back the |
4a8223c to
f7a0f7a
Compare
There was a problem hiding this comment.
Thanks for these changes.
Irritatingly, as a result of the change to jules-shared/jules-model-environment, I think you do now actually need to update the dependencies file in the UM, and provide an upgrade macro to un-ignore l_bgc_heat from UM apps.
Approving this PR, pending Maggie's code owner approval and the creation of a UM PR with an "empty" upgrade macro.
PR Summary
Updating l_bgc_heat to work with UM/and one layer soil carbon model
Sci/Tech Reviewer: Joe Clarke (@josephjclarke)
Code Reviewer: Erica Neininger (@ericaneininger)
https://github.com/MetOffice/um/pull/126
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?UM only linked PR: https://github.com/MetOffice/um/pull/126
Testing
Im putting a series of changes in all of which I will protect with a rose stem test gong forwards
Test Suite Results - jules - l_bgc_heat/run2
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