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

FMS mixed mode #1566

Merged
merged 65 commits into from
May 3, 2022
Merged

Conversation

jiandewang
Copy link
Collaborator

@jiandewang jiandewang commented Apr 11, 2022

this PR contains code change in MOM_diag_manager_infra.F90 to allow UFS to run with FMS mixed mode (MOM6 at 64 bites but fv3 at 32 bites), see detail at
NOAA-EMC#88
The purpose of this PR is asking MOM6 users to test the updated MOM6 code here with the current FMS being used in their system to make sure there will be no issue or answer change.

pjpegion and others added 30 commits May 5, 2020 08:22
update with NOAA-EMC fork
Update ocn_stoch branch with dev/emc
remove conflict with dev/emc
further resolve conflict
merge dev/emc into ocean stochastic branch
put id_sppt_wts, etc back.
@junwang-noaa
Copy link
Contributor

junwang-noaa commented Apr 13, 2022

@sanAkel @thomas-robinson I want to clarify that we have tested the code updates in this PR with following fms versions:

2021.03
2021.04
2022.01
the mixed mode FMS version: https://github.com/NOaA-EMC/FMS/tree/fms_mixedmode.

In the latest ufs-weather-model develop branch, we are using MOM6 with the code updates (in EMC fork) with FMS 2021.04 and we are moving to the fms 2022.01 in the upcoming PR this week.

So committing this MOM6 PR does not require the FMS/dycore updates with mixed mode changes. Instead, once this PR is committed, we can move on to update FMS/dycore with mixed code code changes. When these mixed mode changes are committed/released, MOM6 group can update to the new FMS at their convenient time. Please let me know if you have any questions. Thanks

@jiandewang
Copy link
Collaborator Author

@sanAkel @thomas-robinson I want to clarify that we have tested the code updates in this PR with following fms versions:

2021.03 2021.04 2022.01 the mixed mode FMS version: https://github.com/NOaA-EMC/FMS/tree/fms_mixedmode.

In the latest ufs-weather-model develop branch, we are using MOM6 with the code updates (in EMC fork) with FMS 2021.04 and we are moving to the fms 2022.01 in the upcoming PR this week.

So this MOM6 PR does not have dependency on the mixed mode FMS code updates. Please let me know if you have any questions. Thanks

The purpose of this PR is asking users to test the updated MOM6 code with the current FMS being used in their system to make sure there will be no issue or answer change.

@thomas-robinson
Copy link
Contributor

OK thanks everyone for moving this forward.

We originally slated these updates in FMS for 2022.01, but we ultimately could not take them because then the GFDL climate models would not be able to run with the latest FMS due to updates needed in FV3 and MOM6. All of the necessary changes need to be prepared at the same time, so we will update FMS when the other repositories are ready to update and the GFDL climate models will all be able to run with updated code.

@MinsukJi-NOAA
Copy link

MinsukJi-NOAA commented Apr 13, 2022

@sanAkel and @MinsukJi-NOAA The merge NOAA-GFDL/FMS#898 was reverted because it needs to be coordinated with updates to MOM6 and GFDL_atmos_cubed_sphere.

pray @thomas-robinson, in that case, how should we be exercising the proposed changes in this PR?

We need to come up with a strategy for this. In FMS, I did

git revert 6c3d531c809238ae002c6daac15ea4cb905a7e78

But this created some conflicts that need to be resolved. I think we need an issue in FMS to discuss how to move forward in FMS. Then probably @MinsukJi-NOAA will need do the git revert above and bring the branch up to date with main.

I will look into reverting and bringing the mixed FMS up to date with GFDL main.
However, as @junwang-noaa pointed out, this is an issue that is separate from this MOM6 PR.

@junwang-noaa
Copy link
Contributor

@thomas-robinson Thanks for the information. The changes in this PR are actually different from the original changes proposed for fms 2022.01 previously planned, which had dependency on the fms library. The changes here allow MOM6 to work with current fms versions and the fms version with fix mode updates. So MOM6 changes can be updated without updating fms.

@klindsay28
Copy link
Contributor

It seems like this PR introduces some unneeded duplication of code, with multiple calls to send_data_fms that only differ in whether or not certain optional arguments are passed. It looks like these calls can be collapsed into a single call where the arguments are passed, relying on Fortran's feature of passing optional arguments through to subroutines and/or functions where they are optional, whether or not they are present. Or is there some reason to have the separate calls that I'm not seeing?

@MinsukJi-NOAA
Copy link

It seems like this PR introduces some unneeded duplication of code, with multiple calls to send_data_fms that only differ in whether or not certain optional arguments are passed. It looks like these calls can be collapsed into a single call where the arguments are passed, relying on Fortran's feature of passing optional arguments through to subroutines and/or functions where they are optional, whether or not they are present. Or is there some reason to have the separate calls that I'm not seeing?

These were due to compiler bugs (both intel and gnu) which were documented here:

@junwang-noaa
Copy link
Contributor

@sanAkel @marshallward @kshedstrom @abozec @gustavo-marques May I ask if you have any question with this PR and when it can be committed? The UFS mix-mode project is waiting for the code changes to be committed in MOM6 so that we can move on with the FMS mixed mode PR. Thank you very much!

@marshallward
Copy link
Collaborator

@junwang-noaa Sorry for the long delay, we have no issue with the PR. We are still waiting on regression testing, but we will approve it after it passes (as we expect).

We were a bit concerned about the commit history but have decided today to accept it as-is, in order to allow EMC to preserve the history of its dev/emc branch.

However, we would like to follow up in the near future about potential strategies for management of commit histories across the groups. I will reach out soon to the repo maintainers with more information.

@alperaltuntas alperaltuntas self-requested a review April 25, 2022 20:32
Copy link
Collaborator

@abozec abozec left a comment

Choose a reason for hiding this comment

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

COAPS approves

Copy link
Collaborator

@alperaltuntas alperaltuntas left a comment

Choose a reason for hiding this comment

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

NCAR test are passing.

@marshallward
Copy link
Collaborator

@gustavo-marques Just FYI that we only need one approval per group.

Note to maintainers that we still need approval from @sanAkel before merging.

@gustavo-marques
Copy link
Collaborator

Thanks, @marshallward. I approved because I am listed as one of the reviewers, but good to know.

@sanAkel
Copy link
Collaborator

sanAkel commented Apr 28, 2022

@gustavo-marques Just FYI that we only need one approval per group.

Note to maintainers that we still need approval from @sanAkel before merging.

Thanks for your patience! I will try to get back by Monday next week.

Copy link
Collaborator

@sanAkel sanAkel left a comment

Choose a reason for hiding this comment

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

This preserves answers for main on GEOSgcm; used FMS1. Thanks for your patience!

@marshallward
Copy link
Collaborator

This has been approved by everyone, so I will merge this on behalf of @jiandewang.

Thanks to everyone, and remember to update your branches!

@marshallward marshallward merged commit bac8031 into mom-ocean:main May 3, 2022
@jiandewang
Copy link
Collaborator Author

thanks for everybody's time and effects to have this done.

marshallward added a commit to marshallward/MOM6 that referenced this pull request May 3, 2022
Note that most of these commits are from previously squashed pull
requests, and this PR is restoring them.

- 6360dbb Merge branch 'main' into main_to_dev
- bac8031 Merge pull request mom-ocean#1566 from jiandewang/EMC-FMS-mixed-mode-20220411
- e532d86 Merge pull request mom-ocean#88 from marshallward/missing_attrib_with_class_bugfix
- d380f1d An alternate fix to class(*) issues with FMS 2022-01
- 8ecf333 Merge pull request mom-ocean#87 from jiandewang/feature/update-to-main-20220317
- ba37f94 Merge remote-tracking branch 'FSU/main' into feature/update-to-main-20220317 this is corresponding to MOM6 main 20220317 commit (hash # 399a7db)
- 44313d9 Merge pull request mom-ocean#85 from jiandewang/feature/update-to-main-20220217
- 966707f Merge remote-tracking branch 'GFDL/main' into feature/update-to-main-20220217 this is corresponding to MOM6 main branch 20220217 commit (hash # 6f6d4d6), which originally based on GFDL-candidate-20220129
- 32c0e1e Merge pull request mom-ocean#81 from jiandewang/feature/update-to-main-20211220
- 9642b1d delete external/OCEAN_stochastic_phyiscs directory as Phil re-coded in external/stochastic_physics directory
- e7c9ada solve minor conflict in mom_cap.F90 mom_ocean_model_nuopc.F90 and MOM_energetic_PBL.F90, add two new files: src/parameterizations/stochastic/MOM_stochastics.F90 and config_src/external/stochastic_physics/stochastic_physics.F90
- 90d5961 Merge pull request mom-ocean#78 from jiandewang/feature/update-to-GFDL-20211019
- fd02017 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20211019
- 36f17eb Merge pull request mom-ocean#72 from pjpegion/ocn_stoch_july2021
- a9a957e return a more accurate error message in MOM_stochasics
- 56bb41e Merge branch 'ocn_stoch_july2021' of https://github.com/pjpegion/MOM6 into ocn_stoch_july2021
- ca2ae1c update to dev/emc
- 14ca4a1 Merge pull request mom-ocean#76 from jiandewang/feature/update-to-GFDL-20210914
- 29016c2 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20210914 merge GFDL main 20210914 commit (hash # c09e199)
- a8577df Merge branch 'NOAA-EMC:dev/emc' into ocn_stoch_july2021
- f8a8e4c update to gfdl 20210806 (mom-ocean#74)
- 16e6af0 update to dev/emc
- 237a510 add comments
- 1b4273d revert logic wrt increments
- 5b2040e add logic to remove incrments from restart if outside IAU window
- c5f2b72 add write_stoch_restart_ocn to MOM_stochastics
- bdf2dc7 doxygen cleanup
- 8bc4acc move stochastics to external directory
- a3fa3a1 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch_july2021
- e4bc007 stochastic physics re-write
- 202cbd4 update to dev/emc
- 61717ee Merge remote-tracking branch 'origin/dev/emc' into ocn_stoch
- 565e0bb remove debug statements
- a4c0411 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 689a73f remove PE_here from mom_ocean_model_nuopc.F90
- 8afe969 clean up of mom_ocean_model_nuopc.F90
- 25ed4fc revert MOM_domains.F90
- b8d9888 place stochastic array in fluxes container and make SPPT specific arrays allocatable
- d984a7e remove stochastics container
- eb88219 clean up of code for MOM6 coding standards
- 6e3ea1b correct coupled_driver/ocean_model_MOM.F90 and other cleanup
- 0b99c1f make stochastics optional
- 85023f8 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 80f9f44 clean up MOM_domains
- 5443f8e remove blank link in MOM_diagnostics
- 1727d9a re-write of stochastic code to remove CPP directives
- 600ebf9 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch
- 6bb9d0b fix non stochastic ePBL calculation
- 1d7ffa3 clean up code
- 040e1f1 Merge pull request #13 from NOAA-EMC/dev/emc
- 2cba995 Merge branch 'dev/emc' into ocn_stoch
- 1dc0f4f Merge remote-tracking branch 'upstream/dev/emc' into dev/emc
- 4bd9b9e clean up debug statements
- 25ed5ef additions for stochy restarts
- a2a374b add stochy_restart writing to mom_cap
- 0c15f4c Update MOM_diabatic_driver.F90
- 167a62e Merge pull request #12 from pjpegion/dev/emc
- bd477a9 Update MOM_diabatic_driver.F90
- 7212400 Update MOM_diabatic_driver.F90
- 7de295c cleanup of code and enhancement of ePBL perts
- cd06356 Merge pull request #11 from NOAA-EMC/dev/emc
- 9896d61 Merge pull request #9 from pjpegion/dev/emc_merge
- 0a62737 Merge branch 'ocn_stoch' into dev/emc_merge
- 3cad1ba Merge pull request #8 from NOAA-EMC/dev/emc
- c2aa2a8 updates from dev/emc
- 182ef34 additions for stochastic physics and ePBL perts
- 671c714 Merge pull request #1 from NOAA-EMC/dev/emc
@junwang-noaa
Copy link
Contributor

Thank you all!

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.