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

Methane should not depend on gridcell-level TWS #658

Open
billsacks opened this issue Mar 12, 2019 · 4 comments
Open

Methane should not depend on gridcell-level TWS #658

billsacks opened this issue Mar 12, 2019 · 4 comments
Labels
blocker another issue/PR depends on this one bug something is working incorrectly science Enhancement to or bug impacting science

Comments

@billsacks
Copy link
Member

I often run into a situation where a change in some unrelated part of the code causes substantial answer changes to various methane variables. My best guess is that this is because of the dependence of methane on gridcell-level total water storage (TWS):

https://github.com/ESCOMP/ctsm/blob/b26c12f6318c7e7b766b2960f46aec7c56df7941/src/biogeochem/ch4FInundatedStreamType.F90#L306-L307

This is problematic for two reasons:

  1. Scientifically, we shouldn't have column-level quantities depending on gridcell-level quantities. The way the code currently operates, a change in, say, the glacier portion of a gridcell causes significant answer changes in the vegetated column in that grid cell.

  2. This causes extra time in going through test results for many tags where we expect small answer changes in some limited hydrology fields. These kinds of tags are common right now, so the current operation of this code causes technical debt.

After some deliberation, I have decided to label this as "bug - other". I do believe that point (1) indicates a bug. However, I doubt this has a substantial impact on the scientific results of the model.

@billsacks billsacks added bug something is working incorrectly type: bug - impacts science and removed bug something is working incorrectly labels Mar 12, 2019
@billsacks
Copy link
Member Author

I'm going to upgrade this to "bug - impacts science" because, the more I think about it, the more (1) feels really scientifically wrong. For example, in a grid cell that is part vegetated and part glacier, I believe the methane parameterization currently depends on the amount of glacial ice that we count in the column-level water budget of glacier columns.

@billsacks billsacks changed the title Methane's dependence on gridcell-level TWS is problematic Methane should not depend on gridcell-level TWS Mar 12, 2019
@billsacks billsacks added tag: bug - impacts science bug something is working incorrectly and removed type: bug - impacts science labels May 24, 2019
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Aug 19, 2019
@billsacks billsacks removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Aug 26, 2019
@billsacks
Copy link
Member Author

Discussion at today's clm software meeting: this should be fixed at some point, but not at a super high priority. It will require retuning the relevant parameters.

@billsacks billsacks added the blocker another issue/PR depends on this one label Aug 26, 2019
@billsacks
Copy link
Member Author

Blocks #659

@billsacks
Copy link
Member Author

This issue has tripped me up again. I was puzzled by the fact that, in my testing for both ctsm1.0.dev075 and the upcoming ctsm1.0.dev077 (330fa21, the key commit being 957a1d4), I saw answer changes outside the affected landunits in test ERP_Ly3_P72x2.f10_f10_musgs.IHistClm50BgcCrop.cheyenne_intel.clm-cropMonthOutput: In the testing for ctsm1.0.dev075, I observed answer changes in FSH for crop columns in 2 grid cells (rather than just lake), and in the testing for ctsm1.0.dev077, I observed answer changes in FSH for some crop and natural veg columns (rather than just urban).

When I reran this test from ctsm1.0.dev074, ctsm1.0.dev075 and the branch for ctsm1.0.dev077 (330fa21), but with finundation_method = 'ZWT_inversion', differences were restricted to the affected landunits (lake or urban).

What I'm still confused about is: In a selection of non-transient tests that I spot-checked, including the long tests ERP_P72x2_Lm36.f10_f10_musgs.I2000Clm50BgcCrop.cheyenne_intel.clm-clm50cropIrrigMonth_interp (very similar to the above test) and SMS_Lm13.f19_g17.I2000Clm50BgcCrop.cheyenne_intel.clm-cropMonthOutput, differences in FSH were limited to the affected landunits (lake or urban). I'm not sure why I only saw this spillover effect in a transient test.

@samsrabin samsrabin added science Enhancement to or bug impacting science and removed bug - impacts science labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker another issue/PR depends on this one bug something is working incorrectly science Enhancement to or bug impacting science
Projects
None yet
Development

No branches or pull requests

3 participants