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

add 05degree blom #377

Merged
merged 17 commits into from
Aug 21, 2024
Merged

Conversation

mvertens
Copy link
Contributor

@mvertens mvertens commented Aug 19, 2024

  • add a 1/2 degree blom grid - also will require CICE and ccs_config PRs and is implemented in NorESM2.5 development branch (https://github.com/mvertens/NorESM/tree/feature/noresm2_5_alpha04_v3.

  • fix in nuopc cap for runid that has too short

  • addition of new tests for short term archiving, 2 degree, intel-oneapi, etc

  • simplification of buildlib since master will now only support nuopc coupling infrastructure and cdeps

  • ran new regression test successfully

  • NOTE: for HAMOCC need (this should be done in another PR) - fdepfile, swaclimfile, rivinfile, ndepfile

@mvertens mvertens marked this pull request as draft August 19, 2024 08:42
@mvertens mvertens marked this pull request as ready for review August 20, 2024 08:38
@mvertens
Copy link
Contributor Author

@matsbn @TomasTorsvik - I'd really like to have this as part of noresm2_5_alpha04 that I'm working on creating this week. Consequently, I thought we could move the HAMOCC capability for the half-degree blom grid to an upcoming PR and get this PR accepted first. I've implemented a new set of regression tests that more fully cover other new grids and archiving/resubmit capability as part of this PR.

@TomasTorsvik
Copy link
Contributor

@matsbn @TomasTorsvik - I'd really like to have this as part of noresm2_5_alpha04 that I'm working on creating this week. Consequently, I thought we could move the HAMOCC capability for the half-degree blom grid to an upcoming PR and get this PR accepted first. I've implemented a new set of regression tests that more fully cover other new grids and archiving/resubmit capability as part of this PR.

I agree it makes sense to deal with HAMOCC capability separately, and not halt the progress on the physics side.

Copy link
Contributor

@TomasTorsvik TomasTorsvik left a comment

Choose a reason for hiding this comment

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

I don't get all the details, but overall it looks fine to me.

I have one question regarding OMP, but it may not be an issue, and it's not specific to 0.5 degree grid.

@@ -401,7 +401,7 @@ subroutine InitializeAdvertise(gcomp, importState, exportState, clock, rc)
if (ChkErr(rc, __LINE__, u_FILE_u)) return
read(cvalue,*) nthrds
endif
!$ call omp_set_num_threads(nthrds)
!$ call omp_set_num_threads(nthrds)
Copy link
Contributor

@TomasTorsvik TomasTorsvik Aug 20, 2024

Choose a reason for hiding this comment

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

For all other OMP calls, the prefix !$omp <something openmp> is used. I find it strange that this is not required here. Is this documented somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. This is not being used at this point. I'll raise it as an issue since as you mention it does not effect this PR.

@matsbn
Copy link
Contributor

matsbn commented Aug 20, 2024

Thanks for this @mvertens !

With the changes to buildlib*, will BLOM still work with an MCT based NorESM?

The changes in #375 are rather significant for the application of the hybrid coordinate, so we should consider if that should be part of a noresm2_5_alpha04 release.

@mvertens
Copy link
Contributor Author

@matsbn - yes - BLOM will still work with an MCT based NorESM - but my understanding (based on talking with @TomasTorsvik ) is that master will no longer be used in noresm2.1.x and noresm2.3.x developments - there are now separate branches for this development. Is this not the case? If this is the case - we should consider removing the mct code from master moving forwards.

@TomasTorsvik
Copy link
Contributor

TomasTorsvik commented Aug 20, 2024

@matsbn , @mvertens
We have branches release-1.4 -> noresm2.0, release-1.5 -> noresm2.1. We also have a tag v1.6.1 -> noresm2_3_develop, but we haven't made a release-1.6 branch yet. Technically, we could decide to abandon MCT on master and just branch off v1.6.1 if further MCT updates are needed.

I think we should discuss this in a BLOM-core meeting before making a decision.

@matsbn
Copy link
Contributor

matsbn commented Aug 20, 2024

It is still very useful in ongoing projects to be able to swap in current master in a MCT based NorESM case. We don't really have separate branches fulfilling this purpose to my understanding. The "release-*" branches potentially could, but was intended for final testing before release and later maintenance, bug fixes or documentation enhancements but not answer-changing. A great discussion point for a BLOM-core meeting!

@matsbn
Copy link
Contributor

matsbn commented Aug 20, 2024

I tried the PR with a NorESM2.0.6 setup and get a buildlib failure. Since I have ongoing projects and numerous development tests on this old NorESM base that could still benefit greatly from master compatibility, this PR should ideally not break it.

@mvertens
Copy link
Contributor Author

@matsbn - sorry about that. I had assumed that we no longer needed the 2.0.6 functionality in master. Its easy for me to add it back in.

@mvertens
Copy link
Contributor Author

@matsbn - I've back out the changes and this should work for you now.

@matsbn
Copy link
Contributor

matsbn commented Aug 20, 2024

It builds now, but some of the modified compsets does not work with NorESM2. I think we should find a way to keep the old compsets in parallel for some time. Maybe the old compsets could be renamed to start with "N2", e.g NOIIAOC20TR -> N2OIIAOC20TR. For this specific compset, also directly creating a case with the complete old compset string "20TR_DATM%JRA_SLND_CICE%NORESM-CMIP6_BLOM%ECO_DROF%JRA_SGLC_SWAV" fails with a shr_stream_findBounds error. I wonder if the removed RUN_STARTDATE modification is the reason.

@mvertens
Copy link
Contributor Author

@matsbn - I'll fix that as well.

@mvertens
Copy link
Contributor Author

@matsbn - I've pushed fixes to the compsets back that you suggested. Hopefully this all works now.

Copy link
Contributor

@matsbn matsbn left a comment

Choose a reason for hiding this comment

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

Works well now (also with ancient NorESM version :) Many thanks!

@mvertens mvertens merged commit 65e0b3e into NorESMhub:master Aug 21, 2024
5 checks passed
@mvertens mvertens deleted the feature/add_05degree_blom branch October 2, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
3 participants