Skip to content

Add imp_physics and landsfcmdl to gaussian sfcanl attributes (#147)#148

Merged
aerorahul merged 1 commit into
NOAA-EMC:developfrom
CatherineThomas-NOAA:feature/imp
Oct 9, 2025
Merged

Add imp_physics and landsfcmdl to gaussian sfcanl attributes (#147)#148
aerorahul merged 1 commit into
NOAA-EMC:developfrom
CatherineThomas-NOAA:feature/imp

Conversation

@CatherineThomas-NOAA
Copy link
Copy Markdown
Contributor

Description

The gaussian grid sfcanl file does not have the needed global attributes for upp. The attributes of imp_physics and landsfcmdl are missing and needed for upp to compute cloud and land fields consistently with the model. Upp assumes default values but the imp_physics default is inconsistent with what is used in GFSv17.

Resolves #147
Refs NOAA-EMC/global-workflow#4113

Type of change

  • Bug fix (fixes something broken)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO

How has this been tested?

Global-workflow CI tests (including cycling) on WCOSS2 as well as standalone UPP tests by @WenMeng-NOAA

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • I have made corresponding changes to the documentation if necessary

@CatherineThomas-NOAA
Copy link
Copy Markdown
Contributor Author

Tagging @BrianCurtis-NOAA @barlage for awareness.

@BrianCurtis-NOAA
Copy link
Copy Markdown

BrianCurtis-NOAA commented Oct 3, 2025

The fix and code changes make sense to accomplish a fix to the problem identified. If you need an "official" review (that will allow merge), make sure you give me the necessary permissions.

On another note (read: learning moment): Is there a need to make the attributes of nc files that deal with the surface consistent across the UFSWM/GFS etc spectrum, or would that cause more problems?

The difference of adding imp_physics and landsfcmdl into the namelist read from fort.41 file is that the non-new namelist attributes are all used in different ways throughout the gaussian_sfcanl.f90 code but the new ones are not used, but just carried along through the nc attributes. I'm not familiar enough with things to know if this is a best practice or something to avoid, so I can't advise either way.

@CatherineThomas-NOAA
Copy link
Copy Markdown
Contributor Author

@BrianCurtis-NOAA - Thanks for taking a look.

Is there a need to make the attributes of nc files that deal with the surface consistent across the UFSWM/GFS etc spectrum, or would that cause more problems?

I generally think consistency across files that are similar types is important, though I don't think there's a strict requirement. At this point in v17 development, I would hesitate to make more changes than necessary, but looking forward to post-v17, consistency across these files should be more closely considered.

The UPP is used for both the forecast and analysis files, so at minimum we should make sure that the files have the needed variables so that UPP can treat the forecast and analysis consistently, which relates to your next comment:

The difference of adding imp_physics and landsfcmdl into the namelist read from fort.41 file is that the non-new namelist attributes are all used in different ways throughout the gaussian_sfcanl.f90 code but the new ones are not used, but just carried along through the nc attributes

This is correct. The gaussian_sfcanl utility itself does not do anything with these variables, which I'm guessing is why they weren't included in the first place. But adding them as attributes to the resulting gaussian grid sfcanl file allows for the UPP to compute certain fields consistently with the model forecast.

@CatherineThomas-NOAA
Copy link
Copy Markdown
Contributor Author

@aerorahul - Any other tests or suggestions?

Copy link
Copy Markdown
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

looks good.

@aerorahul
Copy link
Copy Markdown
Contributor

@aerorahul - Any other tests or suggestions?

Nopes. This works for v17.

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.

Add missing global attributes to gaussian sfcanl history file

3 participants