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

On ufs-community/ccpp-physics Issue #172 #173

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

ShanSunNOAA
Copy link

-- 'tisfc' is no longer modified by the atmospheric model, i.e., allowing the ice model to determine the ice temperature over both sea ice and lake ice;
-- Changing tisfc from intent(inout) to intent(in).

…he coupled UFS ATM output during summer)

-- 'tisfc' is no longer modified by the atmospheric model, i.e., allowing the ice model to determine the ice temperature over both sea ice and lake ice;
-- Changing tisfc from intent(inout) to intent(in).

Co-authored-by: Jun Wang <[email protected]>
@@ -111,7 +110,6 @@ subroutine GFS_surface_composites_pre_run (im, lkm, frac_grid, iopt_lake, iopt_l
if (cice(i) >= min_lakeice) then
icy(i) = .true.
islmsk(i) = 2
tisfc(i) = max(timin, min(tisfc(i), tgice))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you confirm if the intent of tisfc for sea ice will also be applied to lake ice?

Copy link
Author

Choose a reason for hiding this comment

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

Mike Barlage confirmed that it should be the lake ice model to determine the ice temperature, not here in the atmospheric model.

Shan

Copy link
Collaborator

Choose a reason for hiding this comment

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

As the PoC of the CLM lake model, @tanyasmirnova Could you confirm that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mzhangw The CLM model sends ice temperature into tisfc(i) variable. However, the line you point to should not be used with the CLM model. The testing of the CLM model has been done with frac_grid = .false., therefore the specific for the CLM lake model code is in the part of the code when frac_land=.false.:

186 else ! Not ocean and not land
187 is_clm = lkm>0 .and. iopt_lake==iopt_lake_clm .and. use_lake_model(i)>0
188 if (cice(i) >= min_lakeice) then
189 icy(i) = .true.
190 if(.not.is_clm) then
191 tisfc(i) = max(timin, min(tisfc(i), tgice))
192 endif
193 islmsk(i) = 2
194 else
195 cice(i) = zero
196 hice(i) = zero
197 islmsk(i) = 0
198 icy(i) = .false.
199 endif
200 islmsk_cice(i) = islmsk(i)
201 flag_cice(i) = .false.
202 if(is_clm) then
203 wet(i) = .true.
204 if (icy(i)) then
205 tsfco(i) = max(tisfc(i), tgice)
206 endif
207 else if(cice(i) < one) then
208 wet(i) = .true. ! some open lake
209 if (icy(i)) then
210 tsfco(i) = max(tisfc(i), tgice)
211 endif
212 endif
213 endif

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mzhangw @tanyasmirnova Following up on this discussion, so the changes in this PR are OK and don't need to be modified for CLM, correct?

@yangfanglin
Copy link
Collaborator

Shan, could you please attach to this PR the slides you and/or Lydia made to illustrate the problem the PR is addressing ?

@ShanSunNOAA
Copy link
Author

This issue is identified here. The output from my experiment is here.

Copy link
Collaborator

@yangfanglin yangfanglin left a comment

Choose a reason for hiding this comment

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

Shan had discussed these changes with a few EMC developers before opening this PR. Approved.

@grantfirl
Copy link
Collaborator

@ShanSunNOAA Have you run UFS regression tests with this change? Do you expect any baseline changes?

@ShanSunNOAA
Copy link
Author

I haven't run any regression tests. I'd be happy to do so, when it is my turn for the PR. I expect most baselines, if not all, will change.

@grantfirl
Copy link
Collaborator

I haven't run any regression tests. I'd be happy to do so, when it is my turn for the PR. I expect most baselines, if not all, will change.

UFS code managers expect RTs to be run before it is your turn in the queue. Their job is to verify that the code is behaving as you claim, so you need to show proof of running RTs on Hera/Derecho/Hercules via a log that shows which tests fail and those failures need to be expected/explainable from your code changes.

I've started PRs for fv3atm and ufs-weather-model (NOAA-EMC/fv3atm#783, ufs-community/ufs-weather-model#2137). I'll fill out the templates and ask for help if I need it.

@ShanSunNOAA
Copy link
Author

Sounds good. Thanks for the info and the heads up.

@grantfirl
Copy link
Collaborator

@ShanSunNOAA Can you please pull in the latest ufs/dev branch code into your PR branch? Let me know if you need help with this.

@FernandoAndrade-NOAA
Copy link

Testing for #2137 has completed successfully, please continue with the merge process.

@grantfirl grantfirl merged commit cc114f4 into ufs-community:ufs/dev Feb 29, 2024
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