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

Add ustar_prev to while loop in icepack_atmo #341

Merged
merged 1 commit into from
Nov 10, 2020
Merged

Add ustar_prev to while loop in icepack_atmo #341

merged 1 commit into from
Nov 10, 2020

Conversation

proteanplanet
Copy link
Contributor

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

This adds a necessary line for checking for convergence of the ustar loop in icepack_atmo that is missing.
Developer: @proteanplanet
Suggested Reviewers: @eclare108213 @dabail10

This is a non-BFB change. There are no dependencies, and no new test cases are added. Documentation is unchanged.

@@ -271,6 +271,8 @@ psixh ! stability function at zlvl (heat and water)

k = 1
do while (abs(ustar - ustar_prev)/ustar > atmiter_conv .and. k <= natmiter)
k = k + 1
ustar_prev = ustar
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a bug fix! Thanks for catching this. Why are these two lines not at the end of the loop though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ustar_prev = ustar can't be at the end of the loop, because it would not be ustar_prev if it was. It doesn't matter if k=k+1 is at the start or end of the loop, but I have changed this so that it is consistent with the ocean coupling routine while loop now in CIME.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct! Looks good. Approving.

@@ -271,6 +271,8 @@ psixh ! stability function at zlvl (heat and water)

k = 1
do while (abs(ustar - ustar_prev)/ustar > atmiter_conv .and. k <= natmiter)
k = k + 1
ustar_prev = ustar
Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct! Looks good. Approving.

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.

Thanks for catching that, @proteanplanet

@apcraig apcraig merged commit 782a1b7 into CICE-Consortium:master Nov 10, 2020
@apcraig
Copy link
Contributor

apcraig commented Nov 16, 2020

The code change seem to be mostly bit-for-bit but comprehensive test results can be seen here,

https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash#782a1b7d78ab0771d33aa411faeb8f06da657b92

apcraig added a commit to apcraig/CICE that referenced this pull request Dec 2, 2020
Add floediam and hfrazilmin as CICE namelist.  Requires latest
version of Icepack.  Update documentation.

Icepack has two non bit-for-bit changes including
  - a fix to multiple declarations of floeshape (CICE-Consortium/Icepack#342)
  - a fix to the convergence scheme in icepack_atmo (CICE-Consortium/Icepack#341)
eclare108213 pushed a commit to CICE-Consortium/CICE that referenced this pull request Dec 3, 2020
Add floediam and hfrazilmin as CICE namelist.  Requires latest
version of Icepack.  Update documentation.

Icepack has two non bit-for-bit changes including
  - a fix to multiple declarations of floeshape (CICE-Consortium/Icepack#342)
  - a fix to the convergence scheme in icepack_atmo (CICE-Consortium/Icepack#341)
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.

4 participants