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

Send broadband fswthrun by categories to coupler. #495

Merged
merged 2 commits into from Jul 31, 2020

Conversation

dabail10
Copy link
Contributor

@dabail10 dabail10 commented Jul 24, 2020

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    This just introduces two new variables, send_i2x_per_cat and fswthrun_ai for sending to the coupler or not.
  • Developer(s):
    dabail10 (D. Bailey)
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_mach_forks#cheyenne
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:
    This is an internal flag that is activated by the cap/driver and not a namelist variable. If the coupler requires this then it will turn it on. Another update is coming from the NUOPC cap that uses this. This is related to issue Send fswthru by categories to coupler #482.

@apcraig
Copy link
Contributor

apcraig commented Jul 24, 2020

Are changes needed in the cap as well? How will send_i2x_per_cat be set? send_i2x_per_cat is a very CESM-centric name. Would something like "save_fswthrunByCat" or similar be better?

@dabail10
Copy link
Contributor Author

The cap will receive a flag from the coupler/mediator calling for the fluxes by category. I will update the NUOPC cap in a separate PR. The CICE standalone cap does not need to be updated. I am happy to rename the logical flag.

@apcraig
Copy link
Contributor

apcraig commented Jul 24, 2020

@dabail10, sounds good. I'd wait for some other reviews before deciding on the name of the flag.

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

I'm okay with send_i2x_per_cat. It's understandable (at least for those of us with some CESM experience) and it's general so that if other variables need to be sent, the same flag can be used.

Where is it declared? Should ice_init.F90 be here too?

@dabail10
Copy link
Contributor Author

send_i2x_per_cat is declared in ice_flux.F90 and set to .false. by default. It is not meant to be read in via the namelist.

@apcraig
Copy link
Contributor

apcraig commented Jul 31, 2020

@eclare108213, @dabail10 is this ready to approve/merge?

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

I don't fully understand how this works. It looks like it's hardwired, but I guess the NUOPC cap overwrites it somehow? I'll approve, since both @dabail10 and @apcraig seem to understand what's going on.

@apcraig
Copy link
Contributor

apcraig commented Jul 31, 2020

@eclare108213, it's hardwired for now. The changes in the NUOPC cap still need to be implemented to turn this feature on.

@apcraig apcraig merged commit 6a3e60c into CICE-Consortium:master Jul 31, 2020
@dabail10 dabail10 deleted the sendbycat branch March 24, 2021 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants