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

PIO standalone capability and IO testing additions #447

Merged
merged 9 commits into from
May 22, 2020

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented May 18, 2020

PR checklist

  • Short (1 sentence) summary of your PR:
    PIO standalone capability and IO testing additions

  • Developer(s):
    apcraig

  • 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.
    Full test suites on izumi, cheyenne, conrad, and gordon look good. This includes new results for cheyenne on pgi and gnu and with the new io_suite on cheyenne. There is a problem with bgc on cheyenne with pgi but I would like to defer that (I never trust the pgi compiler).
    https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#a900320512801bd1389ec35a5da62b49e13c2359
    https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#54315606c8be82c4c9a034111b44963f80df173e

  • 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 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 provide any additional information or relevant details below:

  • Add PIO standalone capability on cheyenne

    • ability to test pio1.10.1, pio2.4.4 thru io_pio2
    • ability to turn on netcdf and pnetcdf thru the restart_format namelist
    • add new settings iopio1, iopio2, iopio1p, iopio2p to support above testing
  • Merge io_pio 'setframe' interface difference into io_pio2 with a cpp (CESM1_PIO) with plans to deprecate io_pio. io_pio is used in RASM and testing in RASM with io_pio2 is underway.

  • Port to gnu and pgi on cheyenne

  • Add histall setting to turn on all history fields

  • Add io_suite to comprehensively test various IO packages

  • Fix a bunch of IO bugs detected by additional IO testing

  • Fix a bug in the precision implementation in iobinary

  • Clean up restart_format implementation

  • Clean up some log output

  • Clean up pio references in Macros files

  • Update documentation

Addresses #246 and #247

Prior to merging, we need to

  • Merge Precision #446 first and test as this may have some conflicts
  • Run a full test suite on cheyenne with the final mods/merge
  • Test with io_pio2 in CESM (one D test checked)
  • Test with io_pio2 in RASM (several configurations verified, others still testing)

@apcraig
Copy link
Contributor Author

apcraig commented May 18, 2020

@dabail10 I want to test this in CESM. Can you tell me what/how to checkout CESM and what tests to run to confirm io_pio2 is working.

@dabail10
Copy link
Contributor

We will need to wait a bit for the NUOPC cap changes to test CICE6 in CESM2. I think we will want to stick with the DTEST (active sea ice only) for now.

@apcraig
Copy link
Contributor Author

apcraig commented May 18, 2020

@dabail10, what does "a bit" mean, a day or a month? Is there a version of CESM (maybe a month or two old) that can run with the current CICE6 code? If we can't test, then we may just have to merge and test later. That's not horrible, I was just hoping we might do some preliminary testing in CESM. That's more or less what I'm doing in RASM, just some basic testing. I'll do more comprehensive testing when I'm ready to update CICE6 in RASM (which will probably align with the next CICE release).

@dabail10
Copy link
Contributor

A few days is what I understand. However, we could probably still test in a version of CESM. I need instructions on how to test up a case with NUOPC running. I believe it is just a create_newcase option to turn on NUOPC.

@apcraig
Copy link
Contributor Author

apcraig commented May 18, 2020

Full tests suites run on cheyenne including the io_suite is here,

https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#abf61997f8e4e48aef6954109ee269d22fe635d8

Everything is working and bit-for-bit except bgc with the pgi compiler. I am going to put that on the back burner for now as I believe it's a pgi issue. Will still need to do this again with final mods.

…_pgi, fix several bugs in history implementation, add histall setting, add io_suite
… cheyenne, add support for pio netcdf and pnetcdf testing, clean up Macros files pio settings, clean up some log output
@apcraig
Copy link
Contributor Author

apcraig commented May 21, 2020

I believe this is ready to merge. I have updated the test results in the template.

Full test suites on izumi, cheyenne, conrad, and gordon look good. This includes new results for cheyenne on pgi and gnu and with the new io_suite on cheyenne. There is a problem with bgc on cheyenne with pgi but I would like to defer that (I never trust the pgi compiler).
https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#a900320512801bd1389ec35a5da62b49e13c2359
https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#54315606c8be82c4c9a034111b44963f80df173e

I have tested in RASM and there have been some preliminary testing in CESM with mixed results. The RASM G cases run fine with io_pio2, the RBR cases seem to have a problem. More effort needs to be spent looking at that issue. RASM can continue to use the io_pio directory for production and further testing will be done asap. Queue wait times have been relatively long in RASM, so testing is moving slowly. CESM does not seem to run properly with io_pio2, but it may never have. This needs to be looked into further as well. I believe we should merge this now and continue testing io_pio2 in coupled mode. The standalone implementation is robust and io_pio2 runs well on cheyenne in standalone mode with pio1.10.1 and pio2.4.4 with both netcdf and pnetcdf for a comprehensive test suite (io_suite).

@apcraig apcraig marked this pull request as ready for review May 21, 2020 16:36
@apcraig
Copy link
Contributor Author

apcraig commented May 21, 2020

I know there is a lot here, but If we could get this merged by Friday evening, we could carry out further comprehensive testing over the weekend as part of weekend testing. This fixes a bunch of bugs and allows for a lot of new io testing on cheyenne.

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.

This is great, @apcraig. I looked through all the changes and nothing jumped out at me. If there are things that still aren't fixed, maybe just document those as comments in this PR, keep going with your testing, and then move whatever is left to be fixed to other issues soon thereafter. Since the pio implementation wasn't working anyhow, I'm not concerned that there may still be things in this PR that aren't yet completely fixed.
My only question is why we have both pio and pio2. Is that temporary, or is it explained somewhere?

@apcraig
Copy link
Contributor Author

apcraig commented May 22, 2020

I have a one line fix for the CESM problems. I am running a full test suite of CICE standalone on cheyenne now (it shouldn't affect the standalone model, but out of an abundance of caution) and then will commit that fix to the branch.

io_pio supports RASM. RASM uses an older version of pio with one interface different. Part of the changes in io_pio2 are there to support RASM, but I still need to verify this in RASM. Actually, I have successfully built and run several cases in RASM with io_pio2, but I need to do a bit more to be 100% certain. But I think I will remove io_pio now from CICE. I can keep io_pio alive in RASM if I need to. It doesn't need to exist in the Consortium version anymore. We are not running or testing io_pio in CICE standalone, so it is just stale code at this point. I will remove io_pio now in my sandbox, run the test suite, then update the PR.

Copy link
Contributor

@dabail10 dabail10 left a comment

Choose a reason for hiding this comment

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

This is now working in CESM2 with PIO2. I agree that we should no longer support PIO1.

@apcraig apcraig merged commit 6daeab5 into CICE-Consortium:master May 22, 2020
@apcraig apcraig deleted the piosa branch August 17, 2022 20:57
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.

3 participants