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

Set gregorian calednar correctly for CICE #112

Merged
merged 1 commit into from
Mar 5, 2024
Merged

Set gregorian calednar correctly for CICE #112

merged 1 commit into from
Mar 5, 2024

Conversation

anton-seaice
Copy link
Contributor

@anton-seaice anton-seaice commented Mar 4, 2024

This change sets the 'time:units' and the 'time:calendar'' attributes of the history output consistent with the nuopc.runconfig file

Patch is cherry-picked from CICE-Consortium/CICE#936

Closes #111

This change sets the 'time:units' and the 'time:calendar'' attributes of the history output consistent with the nuopc.runconfig file

Patch is cherry-picked from CICE-Consortium/CICE#936
@anton-seaice anton-seaice added cice6 Related to CICE6 mediator Related to the CMEPS mediator labels Mar 4, 2024
@anton-seaice anton-seaice self-assigned this Mar 4, 2024
Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

LGTM. I built this successfully and confirmed that CICE history unit and calendar issues are fixed.

I guess this means we no longer need to set use_leap_year in ice_in in the RYF configurations, or is there a reason to keep it?

@anton-seaice
Copy link
Contributor Author

Yeah correct. I captured it in #113 and might do a batch ice_in update at some point?

add_patched_source(OM3_cice CICE/cicecore/drivers/nuopc/cmeps/ice_mesh_mod.F90)
add_patched_source(OM3_cice CICE/cicecore/drivers/nuopc/cmeps/ice_comp_nuopc.F90)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@micaeljtoliveira - is there a neater syntax than this for adding multiple patched sources?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not really. If you have many files to patch then it might be worth putting them in a list and loop over the list contents. Something like this:

list(APPEND FilesToPatch CICE/cicecore/drivers/nuopc/cmeps/ice_mesh_mod.F90 CICE/cicecore/drivers/nuopc/cmeps/ice_comp_nuopc.F90)
foreach(FILE IN LISTS FilesToPatch)
  add_patched_source(OM3_cice ${FILE})
endforeach()

Another option would be to add a variant of add_patched_source that takes multiple files:

add_patched_source(OM3_cice CICE/cicecore/drivers/nuopc/cmeps/ice_mesh_mod.F90 CICE/cicecore/drivers/nuopc/cmeps/ice_comp_nuopc.F90)

IMO, none of them looks much neater than what you have here, but happy to hear other opinions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks - ill leave it alone :)

Comment on lines +48 to +52
+ if (calendar_type == ice_calendar_gregorian) then
+ use_leap_years = .true.
+ else
+ use_leap_years = .false. ! no_leap calendars
+ endif
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be made more compact:

Suggested change
+ if (calendar_type == ice_calendar_gregorian) then
+ use_leap_years = .true.
+ else
+ use_leap_years = .false. ! no_leap calendars
+ endif
+ use_leap_years = calendar_type == ice_calendar_gregorian

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good pickup! I'm also going to leave this as is because we already committed this to cice-main :)

@anton-seaice anton-seaice merged commit 00e4963 into main Mar 5, 2024
2 checks passed
@anton-seaice anton-seaice deleted the issue111 branch March 5, 2024 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cice6 Related to CICE6 mediator Related to the CMEPS mediator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gregorian calendar in CICE
3 participants