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

review comments #117

Closed
wants to merge 1 commit into from
Closed

Conversation

anton-seaice
Copy link

No description provided.

@DeniseWorthen
Copy link

For the UFS side, I think the original placement of the code block in ice_comp_nuopc inside the CESMCOUPLED ifdef would also work, at least for how we intend to use these pio options at this point (via namelist). I think it does no harm (other than generating a warning message) where it is now, but I'm also fine if you revert to the original placement.

One question...the code includes the comment

! The only reason to set these is to detect in ice_history_write if the chunk/deflate settings are ok.

To me, this implies that the same type of checking might not happen if the pio options are set via namelist?

@anton-seaice
Copy link
Author

For the UFS side, I think the original placement of the code block in ice_comp_nuopc inside the CESMCOUPLED ifdef would also work, at least for how we intend to use these pio options at this point (via namelist). I think it does no harm (other than generating a warning message) where it is now, but I'm also fine if you revert to the original placement.

Ah - I didn't think this through. It is in the right place without this PR.

  • If CESMCOUPLED is set, then the iotype comes through NUOPC & CESM shared routines and we need to set the namelist _format options in the driver.
  • If CESMCOUPLED is not set, then format comes through the namelist and is handled in ice_init in the same way as the standalone configuration.

One question...the code includes the comment

! The only reason to set these is to detect in ice_history_write if the chunk/deflate settings are ok.

To me, this implies that the same type of checking might not happen if the pio options are set via namelist?

Its about lines like this:

if (restart_format=='hdf5' .and. restart_deflate/=0) then
         status = pio_def_var_deflate(File, vardesc, shuffle=0, deflate=1, deflate_level=restart_deflate)

in ice_history_write.F90 and ice_restart.F90. If the format is set in the namelist then that if statement works fine too. I will try and reword. Maybe something like 'CICE history and restart handling needs these set to enable chunking / deflate'

@apcraig
Copy link
Owner

apcraig commented Feb 5, 2024

@anton-seaice, please close or revise this PR as needed. Let me know when it's ready.

@anton-seaice
Copy link
Author

@DeniseWorthen , I assume the nuopc driver checks are ok without this change?

@DeniseWorthen
Copy link

@apcraig Yes, thanks. I've been testing w/o the change in ice_comp_nuopc.F90

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.

3 participants