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

Use atparse for variable subsitution in MOM and CICE input files #784

Closed

Conversation

DeniseWorthen
Copy link
Collaborator

@DeniseWorthen DeniseWorthen commented Sep 1, 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

Removes use of edit_inputs for MOM input templates and the ice_in template and uses the atparse function in run_test instead.

Issue(s) addressed

Issue #783

Testing

Initial testing on Cheyenne.intel for all coupled tests passed. All non-standalone tests on hera.intel pass.

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)

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

Dependencies

None

DeniseWorthen and others added 30 commits March 27, 2021 12:30
This reverts commit 7b826d4.
@DeniseWorthen DeniseWorthen added the No Baseline Change No Baseline Change label Sep 2, 2021
@DeniseWorthen DeniseWorthen self-assigned this Sep 2, 2021
@DeniseWorthen DeniseWorthen added the Waiting for Reviews The PR is waiting for reviews from associated component PR's. label Sep 7, 2021
Copy link
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.

Changes are in line with the description and seem correct to the visual review.
I found one place where perhaps the changes did not translate correctly, or were intentional.
Please take a look at the comment.
I would be happy to approve after your response to the comment.

tests/parm/ice_in_template Show resolved Hide resolved
* will be required for C48 coupled case, so add it now
* all tests again pass hera.intel
Copy link
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.

Thanks for the comment.

In the future, if the GODAS application needs this option to be configurable, we can address it then.

@aerorahul
Copy link
Contributor

@DeniseWorthen
I confirmed with NG-GODAS that currently restart_ext = .false. in their application.
However, they wondered if you would be open in keeping this as a configurable option?

@DeniseWorthen
Copy link
Collaborator Author

We have not used this variable in a non-default (.false.) setting at least since updating to CICE6. Yes, we could keep it as a configurable variable, but I do not believe it serves a purpose to do so.

@climbfuji
Copy link
Collaborator

Note that I already pulled this into my PR #792 - no need to update from develop.

@DeniseWorthen
Copy link
Collaborator Author

Will be committed as part of PR #792; closing.

@DeniseWorthen
Copy link
Collaborator Author

Changes will be committed in PR #791; Closing.

@DeniseWorthen DeniseWorthen deleted the feature/parseit branch June 15, 2022 11:58
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 Waiting for Reviews The PR is waiting for reviews from associated component PR's.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants