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

termination threshold due to small cohort number is too high #187

Closed
ckoven opened this issue Feb 22, 2017 · 6 comments
Closed

termination threshold due to small cohort number is too high #187

ckoven opened this issue Feb 22, 2017 · 6 comments

Comments

@ckoven
Copy link
Contributor

ckoven commented Feb 22, 2017

One thing I've noticed in looking at the termination statistics and understory dynamics is that a large fraction of demoted cohorts appear to be getting terminated en route between the canopy and understory when they are carved off from their donor cohorts but before they can be fused into larger understory cohorts of similar size. To address this, one solution is to reduce the minimum number of cohorts before they are terminated. I've pushed a version that includes this change to 3a7b068 on my fork for testing and comparison. These changes qualitatively change the size distribution of understory trees to allow for larger ones to exist, as they appear to allow for successful demotion of these cohorts. This doesn't solve our understory survival problem, as they pretty quickly succumb to carbon starvation anyway, but it may be a necessary step to allow us to address the understory survival issues.

@rosiealice
Copy link
Contributor

rosiealice commented Feb 22, 2017 via email

@ckoven
Copy link
Contributor Author

ckoven commented Feb 22, 2017

very possibly it is an order of operations thing. i can try without the terminate_cohorts in canopy structure to see if that has qualitatively similar effects.

@ckoven
Copy link
Contributor Author

ckoven commented Feb 22, 2017

trying that test now in 4157e60. i left in the call to terminate cohorts that was after the call to fuse them. it is plausible that things still won't survive if they are systematically terminated such that there is never an appropriate cohort to fuse into first, but we'll see if that happens in practice.

@ckoven
Copy link
Contributor Author

ckoven commented Feb 22, 2017

@rosiealice the change in 4157e60 doesn't seem to fix the issue, whereas the one in 3a7b068 does. so it doesn't seem to be primarily an order of operations thing. on the other hand, there's nothing to say that we shouldn't do both sets of changes.

@ckoven
Copy link
Contributor Author

ckoven commented Feb 24, 2017

was wondering if the changes in 4157e60 has any effect at all beyond those in 3a7b068. so i merged them together (37ba5ce) and did a run. below the change in understory biomass from that relative to 3a7b068. clearly there is a change, but it doesn't seem to be very large. will plan on keeping both sets of changes as it seems better to not terminate without fusing first if possible.

understory_biomass_change

@ckoven
Copy link
Contributor Author

ckoven commented Feb 24, 2017

OK, I tried another thing, which was to lower the threshold by 3 more orders of magnitude (e34e368). differencing that from the prior (37ba5ce) looks similar to the prior comparison. its not bit for bit the same, but the changes that emerge (in understory biomass, total number of cohorts, or understory size distributions) do not look qualitatively different from the control. so that suggests that the initial change in the threshold is enough to capture the dynamics that we were missing before. I don't see any benefit though to lowering it to that next level as we do want to eventually terminate cohorts whose number densities are sufficiently small (and also the further reduced threshold seems like it is starting to get close to unsafe math territory). as such, I am going to stick with the changes as of 37ba5ce and issue a PR on that.

bandre-ucar added a commit that referenced this issue Mar 14, 2017
Merge branch 'lower_termination_threshold'

Fixes to termination logic to allow demotion to occur successfully

reduces termination threshold due to small number density and also
deletes calls to terminate_cohorts in CanopyStructureMod except for
those that are after a call to fuse_cohorts.

Fixes: #187

User interface changes?: No

Code review: changes done in consultation with @rgknox and @rosiealice

Testing:

  ckoven

    Test suite: ed tests on lawrencium
    Test baseline: a5dc8da
    Test namelist changes:
    Test answer changes: climate changing

    Test summary: [Remove 'PASS' fields]

        [cdkoven@n0002 ed.lawrencium-lr3.intel.37ba5ce]$ ./cimeteststatus | grep FAIL
        FAIL SMS_D_Lm6.f45_f45.ICLM45ED.lawrencium-lr3_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed.lawrencium-lr3.intel.a5dc8da file)
        FAIL SMS_D_Lm6.f45_f45.ICLM45ED.lawrencium-lr3_intel.clm-edTest.cpl.hi.nc : baseline compare cpl.hi (baseline: compare .base file with ed.lawrencium-lr3.intel.a5dc8da file)
        FAIL SMS_D_Lm6.f45_f45.ICLM45ED.lawrencium-lr3_intel.clm-edTest.rtm.h0.nc : baseline compare rtm.h0 (baseline: compare .base file with ed.lawrencium-lr3.intel.a5dc8da file)
        FAIL SMS_D_Lm6.f45_f45.ICLM45ED.lawrencium-lr3_intel.clm-edTest : baseline compare summary (baseline: compare .base file with ed.lawrencium-lr3.intel.a5dc8da file)

  andre:

    Test suite: ed - yellowstone gnu, intel, pgi
                     hobart nag
    Test baseline: 4ccb9ef
    Test namelist changes: none
    Test answer changes: climate changing, only expected to show up on longer tests
    Test status:
        PASS - all functionality tests.
        Expected baseline failures for all SMS_D_Lm6.f45_f45.ICLM45ED.yellowstone_{gnu,intel,pgi}.clm-edTest.
        PASS - all other tests are bit for bit with baseline.

    Test suite: clm_short - yellowstone gnu, intel, pgi
    Test baseline: clm4_5_12_r195
    Test namelist changes: none
    Test answer changes: bit for bit
    Test status: all tests pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants