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

update icepack #223

Merged
merged 2 commits into from
Nov 2, 2018
Merged

update icepack #223

merged 2 commits into from
Nov 2, 2018

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Oct 31, 2018

Update Icepack to latest version, 0b4ee4e33 Oct 4, 2018

  • Developer(s): tcraig

  • Are the code changes bit for bit, different at roundoff level, or more substantial? NOT bit-for-bit (expected)

  • Does this PR create or have dependencies on Icepack or any other models? No

  • Is the documentation being updated with this PR? (Y/N) No
    If not, does the documentation need to be updated separately at a later time? (Y/N) No

  • Other Relevant Details:

Test results from conrad are here, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks, hash e6b9fee. All tests pass, but answers change, as expected.

@eclare108213
Copy link
Contributor

@apcraig, did you run the QC tests? Since the answers are changing, it would be nice to document the results from that here, now, for future reference.

@apcraig
Copy link
Contributor Author

apcraig commented Oct 31, 2018

I have not run the qc tests. How does that work when all the configurations have changed answers? Do I need to run multiple configurations thru the qc? What configuration should I test?

@eclare108213
Copy link
Contributor

Good question. We should choose a test or set of tests that exercise the part of the code that changed. It looks like there have been 2 Icepack merges that were not BFB since the last update,
CICE-Consortium/Icepack#222 (transport time level inconsistency)
CICE-Consortium/Icepack#227 (shortwave intent inout).
For QC, a single gx3 smoke test was run for 222 but not for 227. I suggest doing the same thing as in 222 (look at the PR) here, and if it does not pass then dive deeper, e.g. run QC for 227 alone.

We need to think about whether we should do other tests from the base suite for QC -- certainly that would be necessary when the changes are in parts of the code that are not exercised in the basic gx3 smoke test. Both of these changes are exercised there, so I think it should be enough for now.

@apcraig
Copy link
Contributor Author

apcraig commented Oct 31, 2018

I'm working on some qc validation. Hope to have some results soon.

@apcraig
Copy link
Contributor Author

apcraig commented Nov 1, 2018

Matt and I are working on the qc testing on conrad, we hit a problem. Sorry for the delay.

@apcraig
Copy link
Contributor Author

apcraig commented Nov 2, 2018

I ran the QC test on the standard configuration compared to the current master and it passed.

Running QC test on the following directories:
/p/work1/apcraig/CICE_RUNS/conrad_intel_smoke_gx1_32x1_medium_qc.qc_base/
/p/work1/apcraig/CICE_RUNS/conrad_intel_smoke_gx1_32x1_medium_qc.qc_test/
Number of files: 1825
2 Stage Test Passed
Quadratic Skill Test Passed for Northern Hemisphere
Quadratic Skill Test Passed for Southern Hemisphere
Quality Control Test PASSED

@mattdturner
Copy link
Contributor

Everything looks good to me. I'll continue working on reducing the memory footprint of cice.t-test.py, but I think this PR is ready to be merged.

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.

approving based on @mattdturner 's comments

@eclare108213 eclare108213 merged commit f5ec75f into CICE-Consortium:master Nov 2, 2018
@apcraig apcraig deleted the ipup05 branch August 17, 2022 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants