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

WW3 to use CPP instead of switches #880

Conversation

JessicaMeixner-NOAA
Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA commented Oct 21, 2021

PR Checklist

  • Ths PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.

  • This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR

  • An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
    are specified below.

  • If new or updated input data is required by this PR, it is clearly stated in the text of the PR.

Instructions: All subsequent sections of text should be filled in as appropriate.

The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsiblity to keep the PR up-to-date with the develop branch of ufs-weather-model.

Description

This PR updates the WW3 submodule to use the latest develop (which will be merged to the dev/ufs-weather-model branch of WW3). The WW3 update is to change from using the WW3 switch fortran pre-processor to using CPP ifdefs instead. The WW3/model/ftn/.ftn code was converted to WW3/model/src/.F90 files. If you have WW3 code that needs to be converted to the new CPP code, use WW3/model/tools/ftn2src.sh to convert your ftn to a src directory. Details about this change can be found in the original WW3 PR: NOAA-EMC/WW3#476

This is a major change for WW3 internal structure, but again no answer changes should occur. This could be combined with non-baseline changing commits, not input or other changes are required.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues must always be created before starting work on a PR branch!)

Testing

How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

Tests were run with an up to date version of ufs-weather-model with hera.intel. There was one difference in point output due to the older version of WW3 being used. This commit should change no answers and no new tests are required. An older version of ufs-weather-model was tested (with the same hera issues) on orion.intel, gaea.intel and wcoss_dell_p3 (no baseline to compare to, but model compiled and ran successfully). This has not been tested on Jet. Denise has compiled this on Cheyenne.

  • hera.intel
  • hera.gnu
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss_cray
  • wcoss_dell_p3
  • CI

Dependencies

This uses the develop branch of WW3 to run the tests instead of the dev/ufs-weather-model

@DeniseWorthen DeniseWorthen added the No Baseline Change No Baseline Change label Nov 2, 2021
@DeniseWorthen
Copy link
Collaborator

We begin committing this PR today. I will pull it into PR #868. Hera is down today for maintenance so we'll run all other platforms today and finish Hera tomorrow.

@DeniseWorthen
Copy link
Collaborator

@JessicaMeixner-NOAA How do you want to mange the update to WW3/dev/ufs-weather-model? Your feature/ww3cpp points to WW3/dev/ufs-weather-model at dd31794 but that is a commit on the WW3/develop branch. Do you want the combined PR to point to WW3/develop @ dd31794 for testing?

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@DeniseWorthen great question. @aliabdolali and I want to be in-line with the preferred strategy for this for ufs-weather-model for this new branch. (Which probably means we probably shouldn't have made the PR from the develop branch- lesson learned on my part for sure). We could go ahead and merge so you are just pointing to the dev/ufs-weather-model or we can point to develop for these tests. @aliabdolali do you have a preference?

@aliabdolali
Copy link
Collaborator

@DeniseWorthen great question. @aliabdolali and I want to be in-line with the preferred strategy for this for ufs-weather-model for this new branch. (Which probably means we probably shouldn't have made the PR from the develop branch- lesson learned on my part for sure). We could go ahead and merge so you are just pointing to the dev/ufs-weather-model or we can point to develop for these tests. @aliabdolali do you have a preference?

I'd prefer mergethe PR to dev/ufs-weather-model, then no change is required on ufs-weather and we can run tests for ufs-weather-model.
Please let me know, I just need to press merge NOAA-EMC/WW3#502

@DeniseWorthen
Copy link
Collaborator

The most natural process from my perspective is to create a PR branch on your WW3 fork to update WW3/dev/ufs-weather-model w/ the current WW3/develop. After testing and merge of WW3/develop to WW3/dev/ufs-weather-model, we'd revert the gitmodules in UWM back from your fork/branch do dev/ufs-weather-model.

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Nov 2, 2021

@aliabdolali The issue w/ that process is that we're updating the commit used for ufs-weather-model prior to testing in the RT suite, right?

Edit: maybe not..sorry!

@JessicaMeixner-NOAA
Copy link
Collaborator Author

The most natural process from my perspective is to create a PR branch on your WW3 fork to update WW3/dev/ufs-weather-model w/ the current WW3/develop. After testing and merge of WW3/develop to WW3/dev/ufs-weather-model, we'd revert the gitmodules in UWM back from your fork/branch do dev/ufs-weather-model.

Definitely should make the PR from a fork next time. I didn't do that because it's not really a development from a fork, it's already in the WW3 develop branch, but in hindsight that doesn't work with PRs. We'll do that better next time unless we want to go ahead and do that now? Whatever is the quickest and easiest and makes sense in terms of not merging before testing, etc.

@DeniseWorthen
Copy link
Collaborator

I think given the amount of change in the commit, I think it would be better not to have WW3/dev/ufs-weather-model updated before the full RT can be run. Could one of you create a branch containing the update to dev/ufs-weather-model that I can point to?

@JessicaMeixner-NOAA
Copy link
Collaborator Author

Working on it-- should have it to you shortly

@JessicaMeixner-NOAA
Copy link
Collaborator Author

New WW3 PR: NOAA-EMC/WW3#515

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@DeniseWorthen there is now a new WW3 PR, this branch is pointing to that new fork/branch (the WW3 commit didn't change though) and this branch is sync'd with the develop of ufs-weather-model.

@DeniseWorthen
Copy link
Collaborator

@JessicaMeixner-NOAA Thanks. The combined PR is now updated. Please check and if all OK, we can start the RTs. I will run the WCOSS machines.

@JessicaMeixner-NOAA
Copy link
Collaborator Author

@JessicaMeixner-NOAA Thanks. The combined PR is now updated. Please check and if all OK, we can start the RTs. I will run the WCOSS machines.

Checked the combined PR and all looks good. Thanks for combining them. Please let me know if there is anything I can do to help in the meantime.

DeniseWorthen added a commit that referenced this pull request Nov 3, 2021
…ate WW3 to use CPP instead of switches (was #880) (#868)


updates NEMS to clean up the setting of Attributes and allow coupled executable to run as standalone ATM
updates WW3 to allow use of CPP ifdefs instead of the WW3 switch fortran pre-processor
 
Co-authored-by:  @JessicaMeixner-NOAA
@DeniseWorthen
Copy link
Collaborator

Code was merged in PR #868. Closing.

@JessicaMeixner-NOAA JessicaMeixner-NOAA deleted the feature/ww3cpp branch November 8, 2021 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Baseline Change No Baseline Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update WW3 to uses CPP instead of switches
3 participants