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.

Sign up for GitHub

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

Jump to bottom

Fixes to methane Tech Note #2091

Merged
merged 6 commits into from
Sep 19, 2023
Merged

Fixes to methane Tech Note #2091

merged 6 commits into from
Sep 19, 2023

Conversation

samsrabin
Copy link
Collaborator

@samsrabin samsrabin commented Aug 9, 2023 •
edited
Loading

Description of changes

Various minor fixes to the methane section of the Tech Note:

  • Added missing citation links
  • Added missing equation references
  • Corrected superscript syntax
  • Corrected units in a table
  • Corrected use of $\phi$ to $\varphi$

Equation references still missing:

  • "The gaseous concentration is calculated with Henry’s law as described in equation ."
  • "… with the same representation as Equation but with the gas diffusivity of oxygen."
  • "The solution to equation is solved in several sequential steps: …"

Specific notes

Contributors other than yourself, if any: @jinmuluo

CTSM Issues Fixed: Item 3 in #1990.

Are answers expected to change (and if so in what way)? No

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

Testing performed, if any: Rendered HTML looks good.

@samsrabin samsrabin added tag: simple bfb documentation additions or edits to user-facing documentation PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete next this should get some attention in the next week or two. Normally each Thursday SE meeting. and removed PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete labels Aug 9, 2023
@samsrabin samsrabin marked this pull request as draft August 9, 2023 19:56
@samsrabin samsrabin self-assigned this Aug 17, 2023
@wwieder
Copy link
Contributor

wwieder commented Aug 22, 2023

@samsrabin I think that:

  1. the gaseous concentration is calculated with Henry’s law as described in equation (2.25.7), since C_{e} is the expected equilibrium gaseous partial pressure.
  2. O2 can also diffuse in from the atmosphere to the soil layer via the reverse of the same pathway, with the same representation as Equation (2.25.8) but with the gas diffusivity of oxygen because A is the diffusive transport (of CH4) through aerenchyma.

@samsrabin
Copy link
Collaborator Author

Thanks, @wwieder!

@samsrabin samsrabin added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Sep 6, 2023
@samsrabin samsrabin marked this pull request as ready for review September 6, 2023 16:54
@samsrabin samsrabin mentioned this pull request Sep 6, 2023
Closed
19 tasks
@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Sep 7, 2023
@samsrabin samsrabin mentioned this pull request Sep 14, 2023
Merged
wwieder
wwieder approved these changes Sep 14, 2023
View reviewed changes
Copy link
Contributor

@wwieder wwieder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making these updates, @samsrabin

doc/source/tech_note/Methane/CLM50_Tech_Note_Methane.rst
@@ -426,7 +426,7 @@ molecular free-air diffusion coefficients (:math:`{D}_{0}`
.. table:: Temperature dependence of aqueous and gaseous diffusion coefficients for CH\ :sub:`4` and O\ :sub:`2`

+----------------------------------------------------------+----------------------------------------------------------+--------------------------------------------------------+
| :math:`{D}_{0}` (m\ :sup:`2` s\ :sup:`-1`) | CH\ :sub:`4` | O\ :sub:`2` |
| :math:`{D}_{0}` (cm\ :sup:`2` s\ :sup:`-1`) | CH\ :sub:`4` | O\ :sub:`2` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming you checked these units with the code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! The constants are set here, where the correct units are given in the comment:
https://github.com/samsrabin/CTSM/blob/7ae1372c0c39747ba71ae399cd2c5964e72f917d/src/main/clm_varcon.F90#L244-L247

When they're used, they're multiplied by 1e-4 to convert D0 from cm2/s to m2/s:
https://github.com/samsrabin/CTSM/blob/7ae1372c0c39747ba71ae399cd2c5964e72f917d/src/soilbiogeochem/SoilBiogeochemNitrifDenitrifMod.F90#L400

However, it looks like the table is missing a digit. Will add.

doc/source/tech_note/Methane/CLM50_Tech_Note_Methane.rst Show resolved Hide resolved
doc/source/tech_note/Methane/CLM50_Tech_Note_Methane.rst Show resolved Hide resolved
samsrabin added a commit that referenced this pull request Sep 19, 2023
@samsrabin
Merge 5 bit-for-bit pull requests:
2e2434d 
* Add system and unit tests for making fsurdat with all crops everywhere (#2081)
* Rework master_list* files etc. (#2087)
* Fixes to methane Tech Note (#2091)
* Add is_doy_in_interval() function (#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (#2079)
* Rework master_list_(no)?fates.rst? (#2083)
* conda run -n can fail if a conda environment is already active (#2109)
* conda fails to load for SystemTests (#2111)
@samsrabin samsrabin merged commit 08e64d0 into ESCOMP:master Sep 19, 2023
2 checks passed
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 19, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into newcropcals-rxsowwindow
8537757 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 20, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into newcropcals-rxsowwindow
ff687dc 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 21, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into newcropcals-rxsowwindow
1382c5d 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 27, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into hist-anomalies
1e1b604 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 2, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into tillage
d43c617 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 3, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into tillage-and-residues3
0d36638 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 4, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into hillslope_hydrology-ssr
595d95d 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 5, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into soil-gas-diffusivity-bugfix
d369f8d 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Dec 23, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into tillage-mike-merge142
19c4790 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)

# Conflicts:
#	src/biogeochem/CNBalanceCheckMod.F90
#	src/biogeochem/CNCIsoFluxMod.F90
#	src/biogeochem/CNDriverMod.F90
#	src/biogeochem/CNPhenologyMod.F90
#	src/biogeochem/CNProductsMod.F90
#	src/biogeochem/CNVegCarbonFluxType.F90
#	src/biogeochem/CNVegNitrogenFluxType.F90
#	src/biogeochem/EDBGCDynMod.F90
#	src/main/clm_initializeMod.F90
#	src/main/controlMod.F90
#	src/soilbiogeochem/SoilBiogeochemDecompCascadeBGCMod.F90
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Dec 23, 2023
@samsrabin
Merge tag 'ctsm5.1.dev142' into tillage-mike-merge142
9cdc75a 
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wwieder wwieder wwieder approved these changes

Assignees

@samsrabin samsrabin

Labels
bfb bit-for-bit documentation additions or edits to user-facing documentation PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samsrabin @wwieder