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

Remove num_solar and implement coszen filters in UrbanAlbedoMod.F90 #2860

Merged
merged 5 commits into from
Nov 10, 2024

Conversation

olyson
Copy link
Contributor

@olyson olyson commented Nov 6, 2024

Description of changes

Remove num_solar and implement coszen filters in UrbanAlbedoMod.F90. Purpose is to fix the issue that albgrd and albgri history fields depend on decomposition, for urban points, and improve performance.

Specific notes

Contributors other than yourself, if any: @billsacks

CTSM Issues Fixed (include github issue #): #17 and #747

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

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

Does this create a need to change or add documentation? Did you do so? No

Testing performed, if any:
aux_clm testing on Derecho indicates bfb with ctsm5.3.009
I haven't done any evaluation of anticipated performance improvements.

@olyson olyson added bug something is working incorrectly bfb bit-for-bit labels Nov 6, 2024
@olyson olyson requested a review from billsacks November 6, 2024 23:06
@olyson
Copy link
Contributor Author

olyson commented Nov 6, 2024

@billsacks , I'm listing you as a reviewer here. Just to make sure that this is what you had in mind regarding solving these two issues.

@olyson olyson added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Nov 6, 2024
Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great - thanks a lot for taking care of this, @olyson ! Yes, this filter addition is what I had in mind.

@wwieder wwieder removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Nov 7, 2024
@wwieder
Copy link
Contributor

wwieder commented Nov 7, 2024

let's merge this on b4b-dev

@olyson olyson changed the base branch from master to b4b-dev November 7, 2024 18:53
@olyson
Copy link
Contributor Author

olyson commented Nov 7, 2024

Testing on Izumi was bfb, with the exception of the Nag compiler mpi-serial failures:

FAIL ERS_D_Ld5_Mmpi-serial.1x1_vancouverCAN.I1PtClm50SpRs.izumi_nag.clm-CLM1PTStartDate RUN time=4
FAIL ERS_D_Mmpi-serial_Ld5.1x1_brazil.I2000Clm50FatesRs.izumi_nag.clm-FatesCold RUN time=14
FAIL SMS_D_Ld1_Mmpi-serial.f45_f45_mg37.I2000Clm50SpRs.izumi_nag.clm-ptsRLA RUN time=4
FAIL SMS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm60FatesRs.izumi_nag.clm-FatesCold RUN time=4
FAIL SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.izumi_nag.clm-default--clm-NEON-HARV RUN time=5
FAIL SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60SpRs.izumi_nag.clm-default--clm-NEON-TOOL RUN time=4

and a couple of tests for which baseline files were not available:

FAIL SMS_Lm3_D_Mmpi-serial.1x1_brazil.I2000Clm50FatesCruRsGs.izumi_intel.clm-FatesColdHydro BASELINE ctsm5.3.009.redo: ERROR BFAIL some baseline files were missing
FAIL ERS_D_Ld15.f45_f45_mg37.I2000Clm50FatesRs.izumi_nag.clm-FatesColdTwoStream BASELINE ctsm5.3.009.redo: ERROR BFAIL some baseline files were missing

However, I've now rebased this to b4b-dev and will repeat testing.
I also have to correct indentation since I left it alone so the PR could be reviewed more easily.

@ekluzek
Copy link
Collaborator

ekluzek commented Nov 7, 2024

However, I've now rebased this to b4b-dev and will repeat testing. I also have to correct indentation since I left it alone so the PR could be reviewed more easily.

@olyson just FYI note there is an checkbox option in github for PR's to "ignore whitespace" which I often find helpful and almost always use it to help with this sort of thing.

@olyson
Copy link
Contributor Author

olyson commented Nov 7, 2024

Oh, awesome!

@olyson olyson merged commit d71b04f into ESCOMP:b4b-dev Nov 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit bug something is working incorrectly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants