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

Fix outgoing lw for history and coupling. #999

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Conversation

dabail10
Copy link
Contributor

@dabail10 dabail10 commented Nov 21, 2024

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:
    A bug was identified when a grid cell goes from ice free to ice covered. Issue Synchronizing of fluxes especially when a point goes from ice free to ice covered. #994
  • 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_hash_forks#6aa677ec49b415298b6368a19e5122f9c9d49f8e
  • 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 update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • 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 document the changes in detail, including why the changes are made. This will become part of the PR commit log.
    This is a two-line change in ice_flux.F90 (scale_fluxes) to check for the condition where aice > ), but flwout > -puny. This means the cell was ice free at the beginning of the step, but non-zero at the end. It sets the outgoing longwave to the open ocean value based on the freezing temperature. This is bfb, but will change the history output of flwup and flwup_ai. Test results are coming.

@dabail10
Copy link
Contributor Author

@anton-seaice
Copy link
Contributor

If its ok, ill defer to Tony and Elizabeth on this. ACCESS don't use this field.

@dabail10
Copy link
Contributor Author

dabail10 commented Dec 2, 2024

If its ok, ill defer to Tony and Elizabeth on this. ACCESS don't use this field.

Really? Was does the atmosphere component use for the upward longwave computation?

@anton-seaice
Copy link
Contributor

Its calculated internally in the atmosphere - presumably in the same way CICE is calculating it

@dabail10
Copy link
Contributor Author

dabail10 commented Dec 3, 2024

Its calculated internally in the atmosphere - presumably in the same way CICE is calculating it

This probably is fine as long as the surface temperature is correct for the sea ice.

@dabail10
Copy link
Contributor Author

While I agree there is a larger issue with timestepping of fluxes, can we agree that this is a suitable fix for the issue #994 ? This sets the LW up based on the local freezing temperature and is only used when aice_init = 0 and aice > 0. Note, that this is the value assigned whenever aice = 0.

@eclare108213
Copy link
Contributor

I don't disagree with the adjusted calculation, but it seems inconsistent to do it in the scale_fluxes routine, in that other fluxes are not being adjusted also. This is fine as long as it's justified. Are the shortwave fluxes already taken care of? Is fixing the other ice fluxes unimportant because they are tiny relative to the LW up from the open ocean fraction? Is there a reason why this LW up value for zero ice isn't initialized at the beginning of the timestep?

@dabail10
Copy link
Contributor Author

The shortwave fluxes and albedo are computed at the end of the step with the updated ice fraction. This is to be synchronized with the shortwave coming from the atmosphere on the next step. However, we could potentially revisit this. I'm doing it in scale_fluxes because it is where flwout is either divided by aice or it is set to the freezing point longwave. I agree there is a larger issue an I am happy to open one for this.

@eclare108213
Copy link
Contributor

flwout is initialized in suboutine init_coupler_fluxes here. This is currently the freshwater value, intended over ice. Would it work to alternatively initialize it here for seawater Tf in an if block based on aice, rather than in scale_fluxes?

@dabail10
Copy link
Contributor Author

This is the whole issue with the bug. It is indeed initialized in init_coupler_fluxes, but then it is overwritten in init_flux_atm to be zero!

@eclare108213
Copy link
Contributor

Then why not initialize it in init_flux_atm? Sorry I'm missing something here, maybe in the order that things are done in the various drivers.

@dabail10
Copy link
Contributor Author

It's an interesting question. The reason things are initialized in init_flux_atm as they are category merged quantities. So, flwout = flwout + sum(aicen(n)*flwoutn(n)). True also of fsens, flat, etc. However, if there is no ice at the beginning of the timestep then it is never computed and hence the issue with the bug. I tried this originally with something like:

where (aice(:,:,:) > 0) then
flwout(:,:,:) = 0
endwhere

Then flwout would only be zeroed out when there was ice at the beginning of the step. This did not quite work as expected though. I don't understand why this was done in both init_flux_atm and init_flux_coupler. My main goal is to fix an actual bug where flwout = c0 when aice > 0. I am open to adding something more general later on.

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 approving this as a bug fix, but please create an issue referencing this PR, to thoroughly examine and document the coupling fields and steps.

@apcraig apcraig merged commit 9c777ac into CICE-Consortium:main Dec 16, 2024
2 checks passed
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.

4 participants