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.

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

Error with calculating o2_decomp_depth in bedrock #93

Closed
ekluzek opened this issue Dec 16, 2017 · 8 comments
Closed

Error with calculating o2_decomp_depth in bedrock #93

ekluzek opened this issue Dec 16, 2017 · 8 comments
Labels
bug something is working incorrectly science Enhancement to or bug impacting science

Comments

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 16, 2017

muszala < bugzillaMuszala > - 2013-06-11 06:02:41 -0600
Bugzilla Id: 1746
Bugzilla CC: erik, muszala, rfisher,

ERS_D.f10_f10.ICLM45BGCNoVS.yellowstone_intel.clm-rootlit

Ran in clm4_5_09. After the refactor in clm4_5_10, this test still runs but is not BFB with clm4_5_09. Consider clm4_5_09 truth.

As a sanity check, ERS_D.f10_f10.ICLM45BGCNoVS.frankfurt_intel.clm-rootlit is BFB between clm4_5_09 and clm4_5_10.

This looks like a problem specific to yellowstone & the intel compiler.

@ekluzek ekluzek added this to the clm5 milestone Dec 16, 2017
@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

muszala < bugzillaMuszala > - 2013-06-12 13:05:28 -0600

found error with nag in

ch4Mod.F90, line 1003: Subscript 2 of POT_F_NIT_VR (value 2) is out of range (1:1).

fix from Jim:: change

if (.not. lake )

t0

if (.not. lake .and. j<=nlevdcomp_full ) then

fixes things with nag+frankfurt, intel+frankfurt and intel+yellowstone.

This will be added in an upcoming tag.

Consider any simulation with this compset suspect until the fix is in place.

Stef-

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

muszala < bugzillaMuszala > - 2013-06-13 09:49:25 -0600

put ch4Mod.F90 fix in clm4_5_12. Tested with:

frankfurt+nag
frankfrut+intel

yellowstone+intel

The fix does change CLM fields for this test only. Coupler and CLM history files remained BFB in all other tests. Please see the clm4_5_12 ChangeLog for details.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

muszala < bugzillaMuszala > - 2013-06-13 10:25:48 -0600

SPM- reopened. proper fix is needs to go in a future tag.

Charlie brought me into this thread. As I mentioned to him, I think what was going on is, the methane model is supposed to work without the vertically resolved BGC (indeed, it was originally developed in that mode), but I didn't anticipate that "NITRIF_DENITRIF" would be used without vertically resolved BGC when I wrote that loop. If this is an unsupported configuration intended for machine testing only, then Jim's fix below is fine; otherwise I would tweak it to get the correct behavior, as Jim's fix will increase anoxia in the top soil layer and reduce methane oxidation more than intended. Better to just ignore the oxygen demand by nitrifiers when the vertically resolved soil BGC is not used, and replace the logic with something like:

"if (.not. lake .and. nlevdcomp >= nlevsoi) then"
or
"if (.not. lake .and. nlevdcomp_full >= nlevgrnd) then"

--Zack

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

Erik Kluzek < erik > - 2014-05-22 23:21:54 -0600

*** Bug 1672 has been marked as a duplicate of this bug. ***

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

Erik Kluzek < erik > - 2014-05-23 00:17:55 -0600

Here's what those changes look like in cesm1_2_2

Index: models/lnd/clm/src/clm4_5/biogeochem/ch4Mod.F90
===================================================================
--- models/lnd/clm/src/clm4_5/biogeochem/ch4Mod.F90	(revision 60566)
+++ models/lnd/clm/src/clm4_5/biogeochem/ch4Mod.F90	(working copy)
@@ -674,7 +674,7 @@
    use ch4varcon,        only : q10ch4base, q10ch4, rootlitfrac, cnscalefactor, mino2lim, & 
                                 f_ch4, lake_decomp_fact, usephfact, anoxicmicrosites, ch4rmcnlim
    use clm_varctl,       only : anoxia
-   use clm_varpar, only : nlevdecomp
+   use clm_varpar, only : nlevdecomp, nlevdecomp_full, nlevgrnd
 #ifdef CENTURY_DECOMP
    use CNDecompCascadeMod_CENTURY, only : nlev_soildecomp_standard
 #else
@@ -999,7 +999,7 @@
 
          ! Add oxygen demand for nitrification
 #if (defined NITRIF_DENITRIF)
-         if (.not. lake) then
+         if (.not. lake .and. nlevdecomp_full >= nlevgrnd) then
             o2_decomp_depth(c,j) = o2_decomp_depth(c,j) + pot_f_nit_vr(c,j) * 2.0_r8/14.0_r8
                                                         ! g N/m^3/s           mol O2 / g N
          end if

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

Erik Kluzek < erik > - 2014-05-23 13:46:50 -0600

Zack's fix was put into: cesm1_2_x_n12_clm4_5_10

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 16, 2017

Bill Sacks < sacks > - 2016-05-16 14:13:44 -0600

Zack's suggested fix never made it to the trunk; it should.

ekluzek added a commit to ekluzek/CTSM that referenced this issue Apr 23, 2018
a465b4f add --quiet argument to improve performance
b2f3ae8 Merge pull request ESCOMP#83 from jedwards4b/jedwards/components_arg
3f4c88f fix comment
c1b5b09 remove unneeded logic
4fdf180 one more test
f78d60f another test
bf52ac6 add a test
91d4851 fix pylint issue
987df5a only use components if populated
98a810d add a components arg to checkout only select components
6923119 Merge pull request ESCOMP#90 from ESMCI/issue-86-detached-sync-status
b11ad61 Merge branch 'master' into issue-86-detached-sync-status
3b624cf Merge pull request ESCOMP#93 from billsacks/work_on_coverage
2562830 Run a single coverage command rather than two separate commands
d1de5f8 Return to starting directory after each test
144f7d9 Merge pull request ESCOMP#92 from billsacks/point_to_esmci
58b8d3e Point to location of repository
0b46d81 Point to correct location for build/coverage status
a385070 fix pylint problems
dcf17b6 make style cleanup
92d342c Rewrite _current_ref to use plumbing rather than parsing porcelain
ca0a5d3 Rework some git repository functions, and major rework of unit tests
719383e Remove commented-out pdb.set_trace() call
376c780 Bugfix: detect and report 'detached from' correctly
21813e9 Add system test demonstrating failure to detect out of sync status.
1a7c59d Merge documentation update into master.
f1e9e99 Merge schema support for git hashes into master.
247fee1 Document return values of checkout.py: main
195c1d0 Implement explicit use of a hash for git repositories.
12dd743 Refactor: schema validation output
fdbc720 Bugfix: incorrect order of operations validing user input

git-subtree-dir: manage_externals
git-subtree-split: a465b4fb740e7895248fab4a302d402ea2ba5cf7
@ekluzek ekluzek removed this from the clm5 milestone Aug 12, 2019
@ekluzek
Copy link
Collaborator Author

ekluzek commented Aug 12, 2019

From looking at the code, it looks like an equivalent fix was put into place. So closing this...

@ekluzek ekluzek closed this as completed Aug 12, 2019
@ekluzek ekluzek changed the title ERS_D.f10_f10.ICLM45BGCNoVS.yellowstone_intel.clm-rootlit error Error with calculating o2_decomp_depth in bedrock Aug 12, 2019
slevis-lmwg pushed a commit to slevis-lmwg/ctsm that referenced this issue Dec 22, 2022
fde04e4 Merge pull request ESCOMP#138 from billsacks/add_python38_tests
37e4c4a Do not update dictionary in-place in loop
7e8474b Remove testing on mac os
7f41c56 Fix pylint issue
3065b0d Add travis-ci tests with python3.7 and python3.8
34fbf55 Add support for git sparse checkout
6c6ef9f Fix pylint errors
6a659ad Added test for sparse checkout and updated documentation
1443243 Support for git sparsecheckout via read-tree.
a48558d Merge pull request ESCOMP#119 from gold2718/submodules
f72ffe7 Do not try git submodule update if no .gitmodules file (git bug)
804e0af Fix a pylint error
45aef95 Addressed review concerns
7da5031 New capability to use git submodule information to checkout externals
1926530 Merge pull request ESCOMP#118 from mnlevy1981/svn_switch
b1b028d Updates after testing
9ea73e6 Add --svn-ignore-ancestry argument
fc5acda Merge pull request ESCOMP#114 from billsacks/fix_large_output_hang
aa2eb71 Try getting travis-ci working on MacOS
96842b4 Fix pylint errors
813fe3c pylint: disable useless-object-inheritance
c49d878 Rework execute_subprocess timeout handling
8fc0e5f Cleanup from 'make style'
b0b23a6 Merge pull request ESCOMP#110 from gold2718/help_fix
3cbcd16 Fixed and clarified help documentation
025e6cb Merge pull request ESCOMP#107 from jedwards4b/ignore_empty_git_dir
489842b if you encounter an empty directory clone into it
0c5a2f6 Merge pull request ESCOMP#106 from billsacks/remove_logfile_message
7799e99 Remove message about checking the log file for more details
0427305 Merge pull request ESCOMP#103 from billsacks/no_logging
9bb46aa Make no-logging be the default
9af6b02 Merge pull request ESCOMP#102 from billsacks/explain_qmark
7f973ae Run through make style
d077a57 Add message describing meaning of '?'
60fc03b Merge pull request ESCOMP#101 from ESMCI/catch_svn_error
28073ec add exception class
4fb7e47 catch errors from svn status --xml
bfa4831 Merge pull request ESCOMP#98 from billsacks/quieter
7d12650 make style
afb4f11 Make more git and svn commands quieter
a465b4f add --quiet argument to improve performance
b2f3ae8 Merge pull request ESCOMP#83 from jedwards4b/jedwards/components_arg
3f4c88f fix comment
c1b5b09 remove unneeded logic
4fdf180 one more test
f78d60f another test
bf52ac6 add a test
91d4851 fix pylint issue
987df5a only use components if populated
98a810d add a components arg to checkout only select components
6923119 Merge pull request ESCOMP#90 from ESMCI/issue-86-detached-sync-status
b11ad61 Merge branch 'master' into issue-86-detached-sync-status
3b624cf Merge pull request ESCOMP#93 from billsacks/work_on_coverage
2562830 Run a single coverage command rather than two separate commands
d1de5f8 Return to starting directory after each test
144f7d9 Merge pull request ESCOMP#92 from billsacks/point_to_esmci
58b8d3e Point to location of repository
0b46d81 Point to correct location for build/coverage status
a385070 fix pylint problems
dcf17b6 make style cleanup
92d342c Rewrite _current_ref to use plumbing rather than parsing porcelain
ca0a5d3 Rework some git repository functions, and major rework of unit tests
719383e Remove commented-out pdb.set_trace() call
376c780 Bugfix: detect and report 'detached from' correctly
21813e9 Add system test demonstrating failure to detect out of sync status.
1a7c59d Merge documentation update into master.
f1e9e99 Merge schema support for git hashes into master.
247fee1 Document return values of checkout.py: main
195c1d0 Implement explicit use of a hash for git repositories.
12dd743 Refactor: schema validation output
fdbc720 Bugfix: incorrect order of operations validing user input
d6423c6 Bugfix: timeout limit for subprocesses
7998f60 Update readme and help output
00b6fb2 Bugfix: add explicit schema version checking
0527869 Update readme
1ae8c84 Merge bugfix branch for stale subexternals into master.
b0c16d7 Bugfix: stale sub-externals after checkout.
30a4e44 Finish implementing system test for mixed-use externals
ac7ff96 Update mixed-use test repo.
bfda7b9 Bugfix: regexp for determining git tracking branches

git-subtree-dir: manage_externals
git-subtree-split: fde04e4d9a758b3aa277aa5fa44a59f5153f2958
samsrabin pushed a commit to samsrabin/CTSM that referenced this issue Apr 19, 2024
### Description of changes

These should have been part of ESCOMP#88 and were used in the testing of that PR but missed in the commit.
@samsrabin samsrabin added bug something is working incorrectly science Enhancement to or bug impacting science labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly science Enhancement to or bug impacting science
Projects
None yet
Development

No branches or pull requests

2 participants