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

change where stem_biomass is set #1284

Closed
ekluzek opened this issue Feb 18, 2021 · 3 comments · Fixed by #1164
Closed

change where stem_biomass is set #1284

ekluzek opened this issue Feb 18, 2021 · 3 comments · Fixed by #1164
Assignees
Labels
code health improving internal code structure to make easier to maintain (sustainability)

Comments

@ekluzek
Copy link
Collaborator

ekluzek commented Feb 18, 2021

These are changes that @olyson is running with for the PPE work. Because stem_biomass should be set to zero in the InitCold method, I think it's equivalent to what's currently being done. But I need to check for sure. This moves the setting of stem_biomass as well as the if woody statement up to where leaf_biomass is set. It also clearly sets stem_biomass to zero there. So it looks like this...

            ! calculate vegetation physiological parameters used in biomass heat storage
            if (use_biomass_heat_storage) then
               ! Assumes fbw (fraction of biomass that is water) is the same for leaves and stems
               leaf_biomass(p) = max(0.0025_r8,leafc(p)) &
                    * c_to_b * 1.e-3_r8 / (1._r8 - fbw(ivt(p)))

               if (woody(ivt(p)) == 1._r8) then
                  stem_biomass(p) = (spinup_factor_deadwood*deadstemc(p) + livestemc(p)) &
                       * c_to_b * 1.e-3_r8 / (1._r8 - fbw(ivt(p)))
               else
                  stem_biomass(p) = 0._r8
               end if
            else
               leaf_biomass(p) = 0._r8
               stem_biomass(p) = 0._r8
            end if
@ekluzek ekluzek added code health improving internal code structure to make easier to maintain (sustainability) next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Feb 18, 2021
@ekluzek ekluzek self-assigned this Feb 18, 2021
@ekluzek
Copy link
Collaborator Author

ekluzek commented Feb 18, 2021

Here's a suggestion from @swensosc to make the code easier to read...

the only thing I don't like about the first version is all the if-blocks, which I think makes it harder to read. If it just started with
leaf_biomass(p) = 0._r8
stem_biomass(p) = 0._r8 before the 'if use bhs' block, we could get rid of the extra else statements and make it cleaner.

leaf_biomass(p) = 0_r8
stem_biomass(p) = 0_r8
if (use_biomass_heat_storage) then                              
                  ! Assumes fbw (fraction of biomass that is water) is the same for leaves and stems 
               leaf_biomass(p) = max(0.0025_r8,leafc(p)) &                      
                    * c_to_b * 1.e-3_r8 / (1._r8 - fbw(ivt(p)))                                                                                            
            if (woody(ivt(p)) == 1._r8) then                                                          
                  stem_biomass(p) = (spinup_factor_deadwood*deadstemc(p) + livestemc(p)) &                       * c_to_b * 1.e-3_r8 / (1._r8 - fbw(ivt(p)))                                                   
               end if
endif

@ekluzek
Copy link
Collaborator Author

ekluzek commented Feb 18, 2021

For code efficiency it's better to use "else" statements rather than set something and then overwrite it for most cases. And actually we should be able to get away with setting biomass to zero just at initialization when use_biomass_heat_storage is off.

@billsacks billsacks removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Feb 18, 2021
@ekluzek
Copy link
Collaborator Author

ekluzek commented Feb 18, 2021

We talked about this a bit, and for this case really the best thing to do is to make sure when BHS is off that biomass is set to zero at initialization. Then it doesn't need to be updated each time-step. There's no real reason we should set this to zero each time-step. That will simplify the code a bit.

As a general philosophy we agreed with the following statement

"Our #1 priority should be code readability, unless we have a strong reason to think that efficiency matters for that part of the code."

ekluzek added a commit to olyson/ctsm that referenced this issue Jun 10, 2021
…eat_storage is false because they are already initialized to zero in InitCold this fixes ESCOMP#1284
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health improving internal code structure to make easier to maintain (sustainability)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants