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

Add fire coupling into ccpp physics including heat flux, upward specific humidity flux, and smoke tracer #193

Merged
merged 7 commits into from
Sep 17, 2024

Conversation

danrosen25
Copy link

@danrosen25 danrosen25 commented Apr 1, 2024

Add heat flux from fire, upward specific humidity flux from fire and the smoke tracer from fire into the ccpp physics package. Use the cpl_fire flag to enable coupling these values.

cpl_fire
hflx_fire
evap_fire
smoke_fire

Co-author: @masih-e

Resolves Issue #192

Testing
One of each compile line in the rt.conf tests was successfully executed on derecho. A new regression test for coupling atm and fire behavior (fbh) was added to rt.conf https://github.com/esmf-org/ufs-weather-model/tree/feature/ufs_fire_cpl

This is a draft pull request until the physics coupling for the community fire behavior model and ccpp physics is complete.

@grantfirl
Copy link
Collaborator

@danrosen25 Any updates on this draft PR?

@danrosen25
Copy link
Author

@grantfirl
We're running through the RTs after some minor changes to make sure everything works. If we get clean results then we'll mark this as ready. The RTs take a long time to run, is there a recommended subset?

@grantfirl
Copy link
Collaborator

@grantfirl We're running through the RTs after some minor changes to make sure everything works. If we get clean results then we'll mark this as ready. The RTs take a long time to run, is there a recommended subset?

@grantfirl We're running through the RTs after some minor changes to make sure everything works. If we get clean results then we'll mark this as ready. The RTs take a long time to run, is there a recommended subset?

@danrosen25 No, unfortunately, they need to pass all of them. Be sure to turn ecflow on and they should run faster. E.g. set -e when running rt.sh.

@mkavulich
Copy link
Collaborator

@danrosen25 Just checking in to see what the status of this PR is, let me know if there's anything I can do to help things along.

@masih-e
Copy link

masih-e commented Jul 3, 2024

Hi, following up with the status of this PR. Please let me know if there is anything I can help with to move things along.

@danrosen25
Copy link
Author

@mkavulich @masih-e
Although the tests passed after the recent sync the "Files Changed" Look wrong. We only touched a few files. I think we'll need to sync using a squash or rebase onto the ufs/dev branch.

@mkavulich
Copy link
Collaborator

mkavulich commented Jul 9, 2024 via email

@masih-e
Copy link

masih-e commented Jul 10, 2024

Michael, would you be able to help with fixing the changes? Please let me know if I can help.

The files that we changed are the following:
GFS_surface_composites_post.*
GFS_surface_generic_post.*
rrfs_smoke_wrapper.*

Thank you both for helping this moving forward

* added hflx_fire and evap_fire to GFS_surface_composite_post
* added cpl_fire flag
* added fsmoke tracer
* added surface emissions to fsmoke in rrfs_smoke_wrapper
@mkavulich
Copy link
Collaborator

@masih-e @danrosen25 Sorry it took so long to get to this, I cherry-picked the appropriate commits and resolved the differences on my branch here: https://github.com/ufs-community/ccpp-physics/compare/ufs/dev...mkavulich:ccpp-physics:feature/ufs_fire_cpl_rebase?expand=1

Apparenly I don't have permission to push to the esmf-org/ccpp-physics repository, so if those changes are correct one of you will have to force-push that branch to this one. Or just open a new PR from a different branch if that's more desirable.

@masih-e
Copy link

masih-e commented Jul 15, 2024 via email

@danrosen25
Copy link
Author

danrosen25 commented Jul 15, 2024 via email

@mkavulich
Copy link
Collaborator

mkavulich commented Jul 16, 2024

Hi Michael, Thank you for doing this. It looks good to me. Should we do the RT again? Masih

I think we should re-run the RTs just to be sure, if that's the last step before opening these PRs for review. Be sure to incorporate this change before running the RTs, since there are some known failures before this point: ufs-community/ufs-weather-model#2362

If you would like help with the RTs just let me know

@danrosen25 danrosen25 marked this pull request as ready for review July 30, 2024 14:49
@danrosen25
Copy link
Author

@grantfirl @Qingfu-Liu @dustinswales @haiqinli @mkavulich
Hi all, this is waiting for review.
Thanks, Dan.

@haiqinli
Copy link
Collaborator

haiqinli commented Aug 5, 2024

@danrosen25 I and Ravan will review it together, and this may take some time. Thanks.

Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

@danrosen25 This looks great to me from a CCPP point of view. Thanks!

@grantfirl
Copy link
Collaborator

@danrosen25 Have the UFS RTs been run with the latest code for this PR? I'm not seeing any logs with https://github.com/ufs-community/ufs-weather-model/pull/2220/files#diff-57a1b7bb92156b1f8a3167f0ba4d34cf3d320188729bd54157b38b1f31a5d2e2. Please let us know if you'd like us to run them with all of your latest PR code from UFS WM on downward.

@danrosen25
Copy link
Author

danrosen25 commented Aug 7, 2024

@grantfirl I didn't check logs into the PR for ufs-weather-model. @masih-e and I individually ran the tests. @masih-e ran them in full but I ran them found a bug, fixed it and then finished the remaining. Can you run the regression tests for verification?
ufs-community/ufs-weather-model#2220
https://github.com/esmf-org/ufs-weather-model/tree/feature/ufs_fire_cpl

@grantfirl
Copy link
Collaborator

@grantfirl I didn't check logs into the PR for ufs-weather-model. @masih-e and I individually ran the tests. @masih-e ran them in full but I ran them found a bug, fixed it and then finished the remaining. Can you run the regression tests for verification? ufs-community/ufs-weather-model#2220 https://github.com/esmf-org/ufs-weather-model/tree/feature/ufs_fire_cpl

@danrosen25 Yes, will do. Assuming success, this PR may be scheduled for merging in the next 2 weeks. I'll let you know.

@grantfirl
Copy link
Collaborator

@danrosen25 Please review/merge in esmf-org#1. This updates your PR branch to the latest ufs/dev so that this can be tested by UFS code managers.

@danrosen25
Copy link
Author

@jkbk2004 @grantfirl
This PR is marked ready because there aren't any submodules that need to be updated. I haven't marked the other PRs because the submodules need to be updated AFTER this PR is merged.

@zach1221
Copy link

@grantfirl testing is complete on WM PR-2220, and we're ready to merge this PR.

@grantfirl grantfirl merged commit b6c4333 into ufs-community:ufs/dev Sep 17, 2024
3 checks passed
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.

7 participants