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

Bug fix in physics/module_sf_noahmp_glacier.F90: only compute glacier albedo in snowalb_bats_glacier if cosz > 0 #1091

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

climbfuji
Copy link
Collaborator

Cherry-picked commit from NRL Github. Verified to resolve the FPE we've run into with NEPTUNE.

@climbfuji
Copy link
Collaborator Author

Resolves ufs-community#227

@climbfuji climbfuji self-assigned this Oct 8, 2024
@dustinswales
Copy link
Collaborator

These changes are fine.
I'm not sure what to make of a PR into this repository that addresses an issue in a fork of this repo. Maybe this change is better suited for ufs-community:ufs/dev?

@climbfuji
Copy link
Collaborator Author

These changes are fine. I'm not sure what to make of a PR into this repository that addresses an issue in a fork of this repo. Maybe this change is better suited for ufs-community:ufs/dev?

As a non-NOAA person, I should be contributing changes from the NRL fork back to the authoritative repository. Remember the fancy diagrams where each organization pushes their changes to the authoritative repo, the other organizations then pull them into their forks when they update from the authoritative repo or if they decide to cherry-pick.

In this case, it's fixing an actual bug, so merging it first into NCAR main and then cherry-picking it for ufs/dev is probably a good idea, unless a full update of ufs-dev from NCAR main is planned in the near future?

@grantfirl
Copy link
Collaborator

@climbfuji @dustinswales I agree with @climbfuji that this is the proper procedure. I do plan on having a PR from NCAR/main into ufs/dev once we're caught up from the other direction, which should be this week.

@dustinswales
Copy link
Collaborator

@climbfuji @grantfirl Agreed, but...
I'm just concerned with this whole process we concocted. Changes to and from the authoritative and fork are resulting in large latencies. This is with two hosts that are both essentially "in house", so I can't see how this would work with more hosts (of course, that's if other hosts ever decide to contribute back to the authoritative...)
The SCM is often (months) behind the UFS and there are external physics PRs into the authoritative that can't proceed until the forks are synced, at which point they need to go through the UFS RT process.

@climbfuji
Copy link
Collaborator Author

But this is the correct workflow. It's probably bad practice from all ends that the updates don't happen as nearly as often as they should. I am going to change this for NRL. My goal is to perform monthly pulls from ncar main and likewise monthly pushes to ncar main. But I want to be in a position that I can anytime, and as often as I need, pull from ncar main with confidence. I am working on the test coverage on NRL's end to make this possible.

@climbfuji
Copy link
Collaborator Author

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants