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

Refactor boxslotcyl advection test to use icepack parameters #358

Merged
merged 4 commits into from
Aug 30, 2019

Conversation

phil-blain
Copy link
Member

PR checklist

  • Short (1 sentence) summary of your PR:
    After I did Advection test - slotted cylinder #254, I realized that two constants I had defined actually already existed in icepack_parameters. I updated the code to use them to reduce duplication.
  • Developer(s):
    Philippe Blain
  • Suggest PR reviewers from list in the column to the right. @eclare108213
  • Please copy the PR test results link or provide a summary of testing completed below.
    base_suite results:
214 of 214 tests PASSED
0 of 214 tests FAILED
0 of 214 tests PENDING
  • 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 (Is this what is meant here ? )
    • 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/.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

@phil-blain
Copy link
Member Author

UCAR FTP is down...

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.

Good catch. Instead of making days_to_s a pointer to secday, please use secday directly. Thanks!

@eclare108213
Copy link
Contributor

Travis is failing on the NCAR ftp. Did the filenames change with the new JRA55 data?

@apcraig
Copy link
Contributor

apcraig commented Aug 30, 2019

Travis should be working. Sometimes, the wget fails (for reasons I don't understand). Usually if you trigger the build again, it works on the 2nd or 3rd try. It looks like someone just triggered the build again.

@apcraig
Copy link
Contributor

apcraig commented Aug 30, 2019

This can probably be merged straight-away. @eclare108213 feel free to do that after you've had another look. Travis worked on 2nd attempt.

@eclare108213 eclare108213 merged commit 1a38c6d into CICE-Consortium:master Aug 30, 2019
@mattdturner
Copy link
Contributor

Sometimes, the wget fails (for reasons I don't understand)

Travis has issues with FTP.

the FTP protocol cannot be reliably used on our sudo-enabled Linux hosted infrastructure anymore.
Source: https://blog.travis-ci.com/2018-07-23-the-tale-of-ftp-at-travis-ci

I know our .travis.yml file has sudo: false included, but that doesn't seem to resolve the issue. It is suggested to use wget with http(s), or sftp to download the files. I'm not sure if that is an option on the NCAR FTP.

@phil-blain phil-blain deleted the refactor-pi-advection branch August 30, 2019 17:23
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