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

Correction of time level inconsistency in transport #222

Merged

Conversation

JFLemieux73
Copy link
Contributor

This PR is for Issue 87 (see description of the problem).

  • Developer(s): Fred Dupont (tests done by JF)

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? not BFB. As a small bug was corrected, we don't expect BFB.

We ran gx3 for 5 years and the changes to the ice and snow are small (hi, aice and hs). After one year of simulation the biggest difference in hi is 2.7 cm and 0.4 cm for hs.

  • Is the documentation being updated with this PR? (Y/N) N. A.
    If not, does the documentation need to be updated separately at a later time? (Y/N)
    Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
    which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-icepack/.

  • Other Relevant Details:

@mattdturner
Copy link
Contributor

I ran the QC test on this PR. The tests all passed.

[08:10]turner@onyx09:./cice.t-test.py $WORKDIR/CICE_RUNS/onyx_intel_smoke_gx3_4x1_medium_qc.PR222_base $WORKDIR/CICE_RUNS/onyx_intel_smoke_gx3_4x1_medium_qc.PR222_test
INFO:main:Number of files: 1825
INFO:main:2 Stage Test Passed
INFO:main:Quadratic Skill Test Passed for Northern Hemisphere
INFO:main:Quadratic Skill Test Passed for Southern Hemisphere
INFO:main:
INFO:main:Quality Control Test PASSED

The process was as follows (mainly included for future reference):

git clone --recursive https://github.com/CICE-Consortium/CICE.git
cd CICE/icepack
git checkout master    # Switch to the master branch
cd ..
./cice.setup -m onyx --test smoke -s qc,medium --testid PR222_base
cd onyx_intel_smoke_gx3_4x1_medium_qc.PR222_base
./cice.build
./cice.submit  # Had to edit PBS info in cice.test to fit within queue limits
cd ../icepack
git remote add JF https://github.com/FJLemieux73/Icepack.git
git fetch JF
git checkout Issue87_TransportSnow   # Checkout the branch for the PR
git branch   # Ensure we are on the new branch
cd ..
./cice.setup -m onyx --test smoke -s qc,medium --testid PR222_test
cd onyx_intel_smoke_gx3_4x1_medium_qc.PR222_test
./cice.build
./cice.submit    # Had to edit PBS info again
cd ..
# Wait for both runs to finish
cp configuration/scripts/tests/QC/cice.t-test.py .
./cice.t-test.py $WORKDIR/CICE_RUNS/onyx_intel_smoke_gx3_4x1_medium_qc.PR222_base $WORKDIR/CICE_RUNS/onyx_intel_smoke_gx3_4x1_medium_qc.PR222_test

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 not sure why the loop needed to be separated into two loops, but I think this is okay. Using the initial values of aicen etc is more consistent, so this is a small bug fix. Not BFB; passes CICE QC tests performed by @mattdturner. This Icepack PR addresses CICE issue 87, CICE-Consortium/CICE#87. Thanks, @dupontf , @JFLemieux73 and @mattdturner !

@eclare108213 eclare108213 merged commit 2fc1efd into CICE-Consortium:master Sep 28, 2018
@dupontf
Copy link

dupontf commented Sep 28, 2018

Thanks a lot Elizabeth!

The reason I have separated the loop in 2 (checks+shift) follows the same logic. The checks are done using aicen and hicen but the latters can already have been updated in case of shifting the ITD up. I could have used the init fields instead but I would have needed to add an additional array hice_init and the final code would have looked much different from the initial one. Of course, either ways (checks and shifting done in the same loop or separated), it would not impact much the response. The separated approach is just more consistent with CICE4 and CICE5.1.2.

lettie-roach pushed a commit to lettie-roach/Icepack that referenced this pull request Oct 18, 2022
…ICE-Consortium#222)

* Read bathymetry file

* Add use_bathymetry flag

* change default ice_in

* Update basalstress documentation

* Remove use_bathymetry from gx1 and gx3 test

* Remove bathymetry_file from alt01 and alt03
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