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

Have a copy of btran2 that's just inside CNFire #1151

Merged
merged 13 commits into from
Sep 23, 2020

Conversation

ekluzek
Copy link
Collaborator

@ekluzek ekluzek commented Sep 15, 2020

Description of changes

Move btran2 so it's just inside of CNFire

Specific notes

Contributors other than yourself, if any: self

CTSM Issues Fixed (include github issue #):

Fixes #1144

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any:
Currently just running the test: SMS_D_Ld3.f10_f10_musgs.I1850Clm50BgcCrop.cheyenne_intel.clm-default
Test builds and runs

@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations code health improving internal code structure to make easier to maintain (sustainability) tag: simple bfb labels Sep 15, 2020
@ekluzek ekluzek added this to the ctsm5.1.0 milestone Sep 15, 2020
@ekluzek ekluzek self-assigned this Sep 15, 2020
…erent for BTRAN2 and BTRAN2FIRE, but that's still the case, so the BTRAN2FIRE calculation needs to be inside CanopyFluxes
…o can output in addition to the standard one
@billsacks
Copy link
Member

It looks like you haven't removed the existing btran2 calculation; is there a reason for that?

@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 16, 2020

I'm still getting this to work. The original btran2 and the new btran2 are different, and I'm trying to track down why and get them to match. Obviously in the end I'll remove the original one and only have the new one. and I'll need to make sure btran2 is only calculated when CN is on.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 16, 2020

Also @billsacks there will be some specific questions I have that I'll want your opinion on. But, I figure I'll wait until this is working correctly. I'll want both you and @negin513 to give me some feedback on how this is done. Mostly in terms of the OO perspective.

@billsacks
Copy link
Member

Okay, sounds good. Let me know when you want my feedback.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 18, 2020

OK, so @billsacks helped me to see the bit-for-bit issue I was having. So I have two design questions I would like to run by @billsacks and @negin513. But, I think what I should do at this point is to get the feedback -- but not change it -- until later. I could do a small refactor later to make this better. I'll just make an issue out of it, and add it to another later bit-for-bit tag.

…N fire module and not avaialble for FATES -- and not needed either
…and only call the subroutine to calculate it when CN is on and FATES is not
…tory and restart back to the original name of BTRAN2/btran2 that was used in EnergyFluxes, now that that version is removed. @billsacks helped me to see that this restart issue is what led to my thinking the results were different
@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 18, 2020

I think this is a good thing, but a consequence of this is that BTRAN2 is dropped off of history files for Sp cases now.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 18, 2020

The following tests are showing answers identical to the baseline:

SMS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm45FatesRsGs.cheyenne_intel.clm-FatesColdDef
ERP_D_Ld3.f09_g17.I2000Clm50SpGs.cheyenne_intel.clm-prescribed
ERI_D_Ld9.f10_f10_musgs.I1850Clm45Bgc.cheyenne_gnu.clm-default
ERI_D_Ld9.f10_f10_musgs.I1850Clm45Bgc.cheyenne_gnu.clm-default
ERI_D_Ld9.f10_f10_musgs.I1850Clm45Bgc.cheyenne_gnu.clm-default
ERS_Ly20_Mmpi-serial.1x1_numaIA.I2000Clm50BgcCropQianRsGs.cheyenne_intel.clm-cropMonthOutput

@billsacks
Copy link
Member

See #1155 for my proposed changes to this PR

@ekluzek
Copy link
Collaborator Author

ekluzek commented Sep 18, 2020

We talked. The plan is that I've pulled this PR into my ctsm5_1 PR and that will come in as it is as the first tag.

@ekluzek ekluzek merged commit 942ec57 into ESCOMP:master Sep 23, 2020
@ekluzek ekluzek deleted the btran2incnfire branch September 23, 2020 07:28
@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) enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move btran2 to just inside of Fire model
3 participants