Skip to content

Feature/mynn sfc submodule#345

Open
XiaSun-Atmos wants to merge 4 commits intoufs-community:ufs/devfrom
XiaSun-Atmos:feature/mynn_sfc_submodule
Open

Feature/mynn sfc submodule#345
XiaSun-Atmos wants to merge 4 commits intoufs-community:ufs/devfrom
XiaSun-Atmos:feature/mynn_sfc_submodule

Conversation

@XiaSun-Atmos
Copy link

@XiaSun-Atmos XiaSun-Atmos commented Jan 14, 2026

Description of Changes:

submodulize mynn sfc layer scheme

Tests Conducted:

rt test using ufs weather model is running now on Ursa
See ufs-community/ufs-weather-model#3052

Dependencies:

No dependencies.

Documentation:

No documentation needed for this PR

Contributors (optional):

@joeolson42 @dustinswales

Copy link

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Is this a drop-in replacement, i.e. is the code the same and are the results identical?

@XiaSun-Atmos
Copy link
Author

The changes are only naming and directory changes. Results are identical.
I ran the full set of rt test for ufs weather model. All tests passed, except for one test that failed to run:
control_p8_ugwpv1_tempo_aerosol_hail intel,
where the mynn sfc scheme is not used in the suite. I am wondering if you have seen this kind of behavior before?
@climbfuji @dustinswales Thanks.

@grantfirl
Copy link
Collaborator

The changes are only naming and directory changes. Results are identical. I ran the full set of rt test for ufs weather model. All tests passed, except for one test that failed to run: control_p8_ugwpv1_tempo_aerosol_hail intel, where the mynn sfc scheme is not used in the suite. I am wondering if you have seen this kind of behavior before? @climbfuji @dustinswales Thanks.

@XiaSun-Atmos Sometimes tests fail due to machine issues; e.g. timing out or write issues. Please upload the logs to ufs-community/ufs-weather-model#3052 and I can take a look.

@climbfuji
Copy link

Thanks @XiaSun-Atmos, it's great that there are no science changes hidden in the reorganization of the files.

@XiaSun-Atmos
Copy link
Author

I uploaded the regression test log ursa. My first round of full rt test log disappeared when I played around with a subset of rt test. The recent round of full rt test just finished.
Thanks @grantfirl @climbfuji

@grantfirl
Copy link
Collaborator

@XiaSun-Atmos @dustinswales This seems to be a little different than the way other schemes have submodulized. This PR puts the CCPP entry point in the submodule as well, whereas other schemes left the CCPP entry point in the main ccpp-physics repo and put the actual scheme files in the submodule. I guess that I don't have a problem with this method, just pointing out that it's different. @dustinswales Any reason not to organize the submodule like this?

@climbfuji
Copy link

@XiaSun-Atmos @dustinswales This seems to be a little different than the way other schemes have submodulized. This PR puts the CCPP entry point in the submodule as well, whereas other schemes left the CCPP entry point in the main ccpp-physics repo and put the actual scheme files in the submodule. I guess that I don't have a problem with this method, just pointing out that it's different. @dustinswales Any reason not to organize the submodule like this?

That's a good point. The idea behind moving the actual scheme to a submodule but leaving the CCPP entry point in the ccpp-physics repo was that people who don't use CCPP can grab just the scheme and don't have to deal with the CCPP overhead. I haven't looked at the files yet and how they are organized. As long as the CCPP entry points only deal with managing input/output data before/after the actual parameterization, and don't do anything else that would make the parameterization disfunctional without it, then that separation of files (and concerns) makes sense and works.

@joeolson42
Copy link
Collaborator

In the long run, we'd like to have a unified driver that can work for all model frameworks. That would make the driver part of the submodule, not left in CCPP. I'd prefer that route so contributions that originate in various modeling frameworks can be handled easier in one unified driver, but yes, there will be some model-specific baggage in there that may be irrelevant for other model frameworks. I still think that is the cleanest path forward. So keep that in mind for any discussions of the position of the entry point. The current driver (in this PR) is purely a CCPP driver, but work is already underway for a unified WRF-MPAS driver. After that is finished (very soon), we will start on merging with the CCPP driver. I hope to have all of this done by end of Feb. Then discussion of the location of this current driver will be obsolete as it will be replaced by the unified driver. Of course, the unified driver will still be CCPP compliant in every other way that I am aware of.

@climbfuji
Copy link

In the long run, we'd like to have a unified driver that can work for all model frameworks. That would make the driver part of the submodule, not left in CCPP. I'd prefer that route so contributions that originate in various modeling frameworks can be handled easier in one unified driver, but yes, there will be some model-specific baggage in there that may be irrelevant for other model frameworks. I still think that is the cleanest path forward. So keep that in mind for any discussions of the position of the entry point. The current driver (in this PR) is purely a CCPP driver, but work is already underway for a unified WRF-MPAS driver. After that is finished (very soon), we will start on merging with the CCPP driver. I hope to have all of this done by end of Feb. Then discussion of the location of this current driver will be obsolete as it will be replaced by the unified driver. Of course, the unified driver will still be CCPP compliant in every other way that I am aware of.

In this case it makes sense to keep the driver in the MYNN submodule, agreed.

@grantfirl
Copy link
Collaborator

In the long run, we'd like to have a unified driver that can work for all model frameworks. That would make the driver part of the submodule, not left in CCPP. I'd prefer that route so contributions that originate in various modeling frameworks can be handled easier in one unified driver, but yes, there will be some model-specific baggage in there that may be irrelevant for other model frameworks. I still think that is the cleanest path forward. So keep that in mind for any discussions of the position of the entry point. The current driver (in this PR) is purely a CCPP driver, but work is already underway for a unified WRF-MPAS driver. After that is finished (very soon), we will start on merging with the CCPP driver. I hope to have all of this done by end of Feb. Then discussion of the location of this current driver will be obsolete as it will be replaced by the unified driver. Of course, the unified driver will still be CCPP compliant in every other way that I am aware of.

Thanks for the context. This makes perfect sense to me.

@grantfirl
Copy link
Collaborator

@XiaSun-Atmos I reran the failing tests on Ursa and they all passed. You should be able to convert the supermodule PRs from draft to regular PRs if you're ready to enter the merge queue.

@XiaSun-Atmos
Copy link
Author

Thanks @grantfirl. I converted the drafts to 'ready for review'.

@gspetro-NOAA
Copy link

@XiaSun-Atmos It looks like you sync'd your WM PR to include the right hashes, but the right CCPP physics hash is not appearing in this sub-PR for some reason, which will cause problems when we try to merge. Perhaps you forgot a push?

@XiaSun-Atmos
Copy link
Author

XiaSun-Atmos commented Feb 13, 2026

@XiaSun-Atmos It looks like you sync'd your WM PR to include the right hashes, but the right CCPP physics hash is not appearing in this sub-PR for some reason, which will cause problems when we try to merge. Perhaps you forgot a push?

@gspetro-NOAA Sorry about that. How about now?

@gspetro-NOAA
Copy link

@gspetro-NOAA Sorry about that. How about now?

Looks good now! Thanks!

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.

6 participants