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

Code is repeated both inside if and in its else statement #1818

Closed
ekluzek opened this issue Aug 2, 2022 · 3 comments · Fixed by #1787
Closed

Code is repeated both inside if and in its else statement #1818

ekluzek opened this issue Aug 2, 2022 · 3 comments · Fixed by #1787
Labels
bfb bit-for-bit code health improving internal code structure to make easier to maintain (sustainability)

Comments

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 2, 2022

The calculation of xmf in SoilTemperatureMod has an if statement where the if and else both contain the same expression. Thus the if statement should just be removed.

@mvdebolskiy noticed this.

                     if (j >= 1) then
                        xmf(c) = xmf(c) + hfus*(wice0(c,j)-h2osoi_ice(c,j))/dtime
                     else
                        xmf(c) = xmf(c) + hfus*(wice0(c,j)-h2osoi_ice(c,j))/dtime
                     endif

It looks like this goes back to this commit:

commit 248307d5bd096a23ff6f85efaf255c0254e0e348
Author: Keith Oleson <oleson@ucar.edu>
Date:   Tue Feb 27 09:25:33 2018 -0700

    Bug fixes for energy imbalance associated with surface water and lakes

Previous to that there was a difference and the else statement included a bit about frac_sno_eff
and looked like this...

xmf(c) = xmf(c) + frac_sno_eff(c)*hfus*(wice0(c,j)-h2osoi_ice(c,j))/dtime

From the commit log this difference was causing an energy imbalance.

@ekluzek ekluzek added code health improving internal code structure to make easier to maintain (sustainability) tag: simple bfb labels Aug 2, 2022
@ekluzek
Copy link
Collaborator Author

ekluzek commented Aug 2, 2022

@olyson and @swensosc does it look correct to you that we should just remove the if and else associated with this statement?

@ekluzek
Copy link
Collaborator Author

ekluzek commented Aug 2, 2022

This issue was noticed @mvdebolskiy in #1787. We actually probably shouldn't change this as there does need to be a difference between the statements in #1787.

@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Aug 4, 2022
@olyson
Copy link
Contributor

olyson commented Aug 4, 2022

I think this was associated with Sean's work to reduce our contribution to the energy imbalance in the CESM2 heat budget as shown in the coupler history logs. As is, we could remove the if/else. If you change this statement then we'll need to keep it in mind when we check coupler energy budgets during CESM3 development to make sure we don't introduce an imbalance from the perspective of the coupled system.

@billsacks billsacks removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Aug 4, 2022
@billsacks billsacks changed the title Code is repeated both inside if and in it's else statement Code is repeated both inside if and in its else statement Jan 17, 2023
@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit 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.

4 participants