Rosie/spitfire cleanup #148

rosiealice · 2016-11-14T21:32:50Z

[ Modifications to turn SPITFIRE on]

[Turns on SPITFIRE by changing the previously always-off hard-wired temporary switch. Makes use_ed_spitfire off by default. Adds a single test for when spitfire is on. Fixes two science bugs in the litter moisture and wind effect algorithms. ]

Fixes: [NGT-ED Github issue #22 ]

User interface changes?: [No]

Code review: [Ben Andre. Rosie Fisher. Jackie Shuman, Ryan Knox ]

Test suite: [ed , clm_short_45, clm_short_50, yellowstone, intel]
Test baseline: [ed-clm-44aac42, clm4_5_12_r195]
Test namelist changes:?
Test answer changes: [changes answers when fire is on (expected)]

Test summary: [grep -e FAIL ~/rosie-sf-status.txt | grep -v summary
FAIL ERS_D_Ld5.5x5_amazon.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL ERS_D_Ld5.f09_g16.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL ERS_D_Ld5.f10_f10.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL ERS_D_Ld5.f19_g16.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL ERS_D_Ld5.f45_g37.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL ERS_D_Mmpi-serial_Ld5.1x1_brazil.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL SMS_D_Ld5.f10_f10.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL SMS_D_Ld5.f45_f45.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL SMS_D_Lm6.f45_f45.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
FAIL SMS_D_Mmpi-serial_Ld5.5x5_amazon.ICLM45ED.yellowstone_intel.clm-edTest.clm2.h0.nc : baseline compare clm2.h0 (baseline: compare .base file with ed-clm-44aac42 file)
CFAIL SMS_Ld5.f10_f10.ICLM45ED.yellowstone_intel.clm-edTest.C.1710163-ed-sfclean
CFAIL SMS_Ld5.f19_g16.ICLM45ED.yellowstone_intel.clm-edTest.C.1710163-ed-sfclean]

…und in SPITFIRE.

…e SPITFIRE test into the test suite.

Merge branch 'master' into rosie/spitfire_cleanup Conflicts: components/clm/cime_config/testdefs/testlist_clm.xml

rgknox · 2016-11-15T18:36:50Z

Thanks Rosie, I will get a review in on this over the next few days.
Review: Implementation seems fine, saw no issues. Will start testing on lawrencium soon.

rgknox · 2016-11-28T08:10:26Z

All tests look good on lawrencium, except for the "edNoFire" test, because that test condition no longer exists. I think we just need to switch that test to the "edFire" test and we should be good to go.
I'd be happy to make the change if no one gets to it before me.

Swapped out the EdNoFire for EdFire test.

rgknox · 2016-11-29T21:11:22Z

This PR looks fine to me. Any others want a review? @bandre-ucar, can you sign off on this too?

bandre-ucar · 2016-11-29T23:27:58Z

Looks fine. I'll test on yellowstone.

bandre-ucar · 2016-11-29T23:48:27Z

Testing on yellowstone and hobart-nag.

bandre-ucar · 2016-11-30T00:01:49Z

components/clm/src/ED/fire/SFMainMod.F90

@@ -56,7 +56,7 @@ subroutine fire_model( currentSite, atm2lnd_inst, temperature_inst)

    !zero fire things
    currentPatch => currentSite%youngest_patch
-    temporary_SF_switch = 0
+    temporary_SF_switch = 1


I was just reviewing the changes for the commit message and noticed the temporary_SF_switch is still here.

Spitfire is runtime configurable and is now off by default. Since it is easy to turn on/off via-name list and this is only used to disable spitfire at compile time, it seems like this should be removed....

rosiealice · 2016-11-30T00:03:31Z

We can definitely get rid of it...

…

On 29 November 2016 at 17:01, Ben Andre ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In components/clm/src/ED/fire/SFMainMod.F90 <#148 (review)>: > @@ -56,7 +56,7 @@ subroutine fire_model( currentSite, atm2lnd_inst, temperature_inst) !zero fire things currentPatch => currentSite%youngest_patch - temporary_SF_switch = 0 + temporary_SF_switch = 1 I was just reviewing the changes for the commit message and noticed the temporary_SF_switch is still here. Spitfire is runtime configurable and is now off by default. Since it is easy to turn on/off via-name list and this is only used to disable spitfire at compile time, it seems like this should be removed.... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#148 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMWsQ-AiKZYKFAS8gOXjyun4qKDT_PY6ks5rDLzugaJpZM4Kx1jn> .

-- ------------------------------------------------------------------------ Dr Rosie A. Fisher Staff Scientist Terrestrial Sciences Section Climate and Global Dynamics National Center for Atmospheric Research 1850 Table Mesa Drive Boulder, Colorado, 80305 USA. +1 303-497-1706 http://www.cgd.ucar.edu/staff/rfisher/

rosiealice · 2016-12-01T17:46:33Z

Is it easy to get this on to the trunk without modifying the switch? Jackie and I are waiting on getting this piece sorted so we can move ahead with fixing all of the issues we found in the SPITFIRE code while comparing it to newer publications, etc. So, we can easily remove the actual code for the hardwired switch during that process?

Remove temporary_SF_switch. Spitfire is runtime configurable via name list and is now off by default. This switch was only used to disable spitfire at compile time. Fixes: 140 User interface changes?: spitfire configured at runtime instead of compile time. Code review: self Test suite: SMS_D_Mmpi-serial_Ld5.5x5_amazon.ICLM45ED.yellowstone_intel.clm-edTest Test baseline: 2ac7960 Test namelist changes: none Test answer changes: bit for bit Test summary: pass

bandre-ucar · 2016-12-01T19:14:39Z

@rosiealice It simpler and clearer in the long run remove it now so the changes related to the switch and tests are all in a single merge. I've submitted a PR to you with the fix. Please accept it and I'll test and merge this PR this afternoon.

Remove temporary_SF_switch

bandre-ucar · 2016-12-01T20:39:12Z

Testing on yellowstone and hobart.

Rosie Fisher and others added 9 commits February 24, 2016 16:40

Changed temporary_spitfire_switch to 1

fdd3505

Modified two bugs in effective windspeed and in livegrass moisture fo…

1d4238a

…und in SPITFIRE.

Merge branch 'master' into rosie/spitfire_cleanup

edac755

turned off SPITFIRE temporary switch for testing

a246d58

Merge branch 'master' into rosie/spitfire_cleanup

dca960c

turn temp_sf_switch on

32bc16a

Altered test scripts to make spitfire off by default, and add a singl…

824e36c

…e SPITFIRE test into the test suite.

Changes to namelist defaults to turn spitfire off

7ece58c

re-added testlist file that acidentally got deleted

c04a117

Merge branch 'master' into rosie/spitfire_cleanup Conflicts: components/clm/cime_config/testdefs/testlist_clm.xml

rgknox and others added 2 commits November 28, 2016 23:26

Swapped out the EdNoFire for EdFire test.

525f309

Merge pull request #1 from rgknox/rosie/spitfire_cleanup

01d60bf

Swapped out the EdNoFire for EdFire test.

bandre-ucar self-assigned this Nov 29, 2016

bandre-ucar reviewed Nov 30, 2016

View reviewed changes

Merge pull request #2 from bandre-ucar/rosie/spitfire_cleanup

2560ea1

Remove temporary_SF_switch

bandre-ucar mentioned this pull request Dec 1, 2016

Fixes: carbon flux naming, messages and warning, thread-change test #147

Merged

bandre-ucar merged commit 2560ea1 into NGEET:master Dec 1, 2016

bandre-ucar removed their assignment Apr 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rosie/spitfire cleanup #148

Rosie/spitfire cleanup #148

rosiealice commented Nov 14, 2016 •

edited by rgknox

Loading

rgknox commented Nov 15, 2016 •

edited

Loading

rgknox commented Nov 28, 2016 •

edited

Loading

rgknox commented Nov 29, 2016

bandre-ucar commented Nov 29, 2016

bandre-ucar commented Nov 29, 2016

bandre-ucar Nov 30, 2016

rosiealice commented Nov 30, 2016 via email

rosiealice commented Dec 1, 2016

bandre-ucar commented Dec 1, 2016

bandre-ucar commented Dec 1, 2016

Rosie/spitfire cleanup #148

Rosie/spitfire cleanup #148

Conversation

rosiealice commented Nov 14, 2016 • edited by rgknox Loading

rgknox commented Nov 15, 2016 • edited Loading

rgknox commented Nov 28, 2016 • edited Loading

rgknox commented Nov 29, 2016

bandre-ucar commented Nov 29, 2016

bandre-ucar commented Nov 29, 2016

bandre-ucar Nov 30, 2016

Choose a reason for hiding this comment

rosiealice commented Nov 30, 2016 via email

rosiealice commented Dec 1, 2016

bandre-ucar commented Dec 1, 2016

bandre-ucar commented Dec 1, 2016

rosiealice commented Nov 14, 2016 •

edited by rgknox

Loading

rgknox commented Nov 15, 2016 •

edited

Loading

rgknox commented Nov 28, 2016 •

edited

Loading