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 buildnml to use the CIME-CCS python based namelist generation capabilities #263

Merged
merged 33 commits into from
Aug 30, 2023

Conversation

mvertens
Copy link
Contributor

@mvertens mvertens commented Jul 25, 2023

This PR refactors the namelist generation in BLOM to use the python based capabilities in CIME. This is the way that CICE, all CDEPS components, CMEPS, WW3DEV and POP generate their namelists. CAM and CTSM still use an older perl based scripting with the intention of moving away from that in the future.

  1. buildnml is now a python based script
  2. there is a new file namelist_generation_blom.xml that is used to generate all of the namelists that are used by blom. The matching functionality is based on the compset, resolution as well as values in a new python dictionary config[] that is introduced in the new buildnml. Entries in this xml are of the following forms:

for real variables:

  <entry id="mdv2hi">
    <type>real</type>
    <category>limits</category>
    <group>limits</group>
    <values>
      <value>.02</value>
      <value blom_unit="cgs">2.</value>
      <value ocn_grid="tnx0.25v4"  blom_unit="cgs">.15</value>
      <value ocn_grid="tnx0.25v4"  blom_unit="mks">.0015</value>
      <value ocn_grid="tnx0.125v4" blom_unit="cgs">.1</value>
      <value ocn_grid="tnx0.125v4" blom_unit="mks">.001</value>
    </values>
    <desc></desc>
  </entry>

for file based variables:

  <entry id="tdfile">
    <type>char</type>
    <category>limits</category>
    <group>limits</group>
    <values>
      <value>UNSET</value>
      <value ocn_grid="tnx2v1">$DIN_LOC_ROOT/ocn/blom/bndcon/tidal_dissipation_tnx2v1_20130419.nc</value>
      <value ocn_grid="tnx1v4">$DIN_LOC_ROOT/ocn/blom/bndcon/tidal_dissipation_tnx1v4_20170605.nc</value>
      <value ocn_grid="tnx0.25v4">$DIN_LOC_ROOT/ocn/blom/bndcon/tidal_dissipation_tnx0.25v4_20170626.nc</value>
      <value ocn_grid="tnx0.125v4">$DIN_LOC_ROOT/ocn/blom/bndcon/tidal_dissipation_tnx0.125v4_20221013.nc</value>
    </values>
    <input_pathname>abs</input_pathname>
    <desc>
      Name of file containing tidal wave energy dissipation divided by
      by bottom buoyancy frequency
    </desc>
  </entry>

Note that <input_pathname>abs</input_pathname> is used to create the Buildconf/blom.input_data_list entries.

for character variables:

  <entry id="swamth">
    <type>char</type>
    <category>limits</category>
    <group>limits</group>
    <values>
      <value>jerlov</value>
    </values>
    <desc></desc>
  </entry>
  1. the Buildconf/blom.input_data_list is automatically created based on the entrires in namelist_generation_blom.xml.
  2. user settings of namelist values can now be done in user_nl_blom via keyword/value settings. This is a big advantage of the current implementation. In addition, checks are made on the user entries in user_nl_blom (i.e. if an integer is expected, a logical cannot be entered, etc).
  3. documentation of all namelist variables is now in ocn_in.readme that is placed along side the generated ocn_in namelist in $RUNDIR.
  4. the older csh based buildnml (without the namelist documentation) is being temporarily kept as a renamed file - buildnml.csh.
  5. this refactor is a critical part of getting regression testing working in BLOM. The reason is that in general daily history files need to be compared both for ecosys and non-ecosys configurations. For comparisons to work - the fields need to be at least single precision. In addition for performance tests - you don't want to have any history files created - hence the empty_hist attribute below. With this refactor - this is easily done via the following type of xml entry:
<entry id="srf_phosph">
    <type>integer(3)</type>
    <category>diabgc</category>
    <group>diabgc</group>
    <values>
      <value>0,2,2</value>
      <value is_test="yes">4,2,2</value>
      <value is_test="yes" empty_hist="yes">0,0,0</value>
    </values>
    <desc>Phosphorus (po4) [mol P m-3]</desc>
 </entry>

Testing:
Used the following create_newcase command out of this code sandbox and a code sandbox that had the previous BLOM namelist generation:

./create_newcase --case <CASENAME> --compset 2000_SATM_SLND_SICE_BLOM%ECO_SROF_SGLC_SWAV_SESP --res T62_tn14 --run-unsupported --project <PROJECT> --mach betzy

For the following different grids verified that the sorted namelists were the same

xmlchange OCN_GRID=tnx2v1
xmlchange OCN_GRID=tnx1v4
xmlchange OCN_GRID=tnx0.25v4
xmlchange  OCN_GRID=tnx0.125v4

For tnx1v4 verified that the sorted namelists were the same:

xmlchange OCN_NCPL=48
xmlchange  BLOM_UNITS=mks
xmlchange BLOM_COUPLING=partial
xmlchange BLOM_COUPLING= partial and BLOM_VCOORD=cntiso_hybrid
xmlchange BLOM_TRACER_MODULE=iage
xmlchange BLOM_NDEP_SCENARIO=2000
xmlchange BLOM_NDEP_SCENARIO=hist
xmlchange BLOM_NDEP_SCENARIO=ssp245

@mvertens mvertens marked this pull request as draft July 25, 2023 07:19
@mvertens mvertens added the enhancement New feature or request label Jul 26, 2023
@mvertens mvertens marked this pull request as ready for review July 26, 2023 14:07
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.

This is quite an extensive PR, but I see that it mostly is a restructuring of how the namelists are generated (through a python script instead of a shell script).

Copy link
Contributor

Choose a reason for hiding this comment

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

This overwrites changes to PR #262. I guess the intention is to remove the HAMOCC_VSLSC option, so PR #263 should be merged after PR #262 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This builds on top of changes to #262. I was hoping to use #262 immediately for the Making Waves project and then
have this PR reviewed more extensively so that everyone was comfortable with it. It would be good to also understand why this PR is important to get regression testing working with the right comparison of BLOM output.
If you were okay with merging #262 - that would be great!

Copy link
Contributor

Choose a reason for hiding this comment

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

I have now merged #262 so it should be available in master. I talked briefly with Jörg, but but I don't know when Mats will be around. Hopefully we can have a meeting soon and discuss the implications of this PR.

@mvertens mvertens force-pushed the feature/refactor_buildnml branch from dcdadcd to 8bade57 Compare August 3, 2023 09:13
@TomasTorsvik
Copy link
Contributor

@mvertens - I tried to set up a NOINYOC run using a run script for noresm2.0.6, but based on the noresm_develop_v7_proto branch for NorESM and this feature/refactor_buildnml branch. Basically, I wanted to check if it was possible to run the same setup as before once we merge this PR into master.

What I find is that the grid T62_tn21 is not defined, and for grid T62_tn14 I get an error that seems to be related to a missmatch in the processor count (see below). I might be doing something wrong in my setup, but it would be good to ensure that we can still run CMIP6 compsets after merging this PR.

/cluster/projects/nn2980k/tomast/NORESM/NorESM-mvertens/components/blom//cime_config/buildlib:6: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import os, shutil, sys, glob, imp
ERROR: Command /cluster/projects/nn2980k/tomast/NORESM/NorESM-mvertens/components/blom/cime_config/buildlib_2.2 /cluster/projects/nn2980k/tomast/NORESM/cases/NOINYOC_T62_tn14_NorESM-mvertens_1y_20230803T1206 /cluster/work/users/toma
st/noresm/NOINYOC_T62_tn14_NorESM-mvertens_1y_20230803T1206/bld/lib /cluster/work/users/tomast/noresm/NOINYOC_T62_tn14_NorESM-mvertens_1y_20230803T1206/bld/ocn/obj failed rc=1
out=...calling blom buildcpp to set build time options
err=/cluster/projects/nn2980k/tomast/NORESM/NorESM-mvertens/components/blom/cime_config/buildlib_2.2:6: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import os, shutil, sys, glob, imp
ERROR: Command /cluster/projects/nn2980k/tomast/NORESM/NorESM-mvertens/components/blom/bld/blom_dimensions -n 1 -k 53 -d /cluster/projects/nn2980k/tomast/NORESM/NorESM-mvertens/components/blom/bld/tnx1v4 failed rc=1
out=
err=blom_dimensions: Cannot find patch.input file for 1 processors!
blom_dimensions: Available processor counts: 32 42 63 77 91 123 156 186 256 354

@TomasTorsvik TomasTorsvik self-requested a review August 3, 2023 10:29
@mvertens
Copy link
Contributor Author

mvertens commented Aug 3, 2023

@TomasTorsvik - we need to add the tn21 grid to ccs_config and also create a mesh for it. I'm happy to do that.
Can you please change your permissions on /cluster/projects/nn2980k/tomast/NORESM/cases/NOINYOC_T62_tn14_NorESM-mvertens_1y_20230803T1206
so that I can see your directory - or have me added to nn2980k? That would be very helpful.

@mvertens
Copy link
Contributor Author

mvertens commented Aug 3, 2023

@TomasTorsvik - you need to use noresm_develop_v7 in order to get the right proessor layout in blom.

I was successfully able to do the following:

> git clone https://github.com/NorESMhub/NorESM.git
> cd NorESM
> git checkout noresm_develop_v7
> ./manage_externals/checkout_externals -v
> cd components/blom
> git remote add mvertens https://github.com/mvertens/BLOM
> git fetch mvertens
> git checkout feature/buildnml_refactor
> cd ../../cime/scripts
> ./create_newcase --case NOINYOC_refactor --compset NOINYOC --res T62_tn14 --project nn9560k --run-unsupported --mach betzy
> cd NOINYOC_refactor
> ./pelayout (this provides a valid pelayout)
> ./case.build (this built the model with no problem)

So I need to add the T62_tn21 and an accompanying test in blom. Should I add any other resolution?

@TomasTorsvik
Copy link
Contributor

@TomasTorsvik - you need to use noresm_develop_v7 in order to get the right proessor layout in blom.

I was successfully able to do the following:

> git clone https://github.com/NorESMhub/NorESM.git
> cd NorESM
> git checkout noresm_develop_v7
> ./manage_externals/checkout_externals -v
> cd components/blom
> git remote add mvertens https://github.com/mvertens/BLOM
> git fetch mvertens
> git checkout feature/buildnml_refactor
> cd ../../cime/scripts
> ./create_newcase --case NOINYOC_refactor --compset NOINYOC --res T62_tn14 --project nn9560k --run-unsupported --mach betzy
> cd NOINYOC_refactor
> ./pelayout (this provides a valid pelayout)
> ./case.build (this built the model with no problem)

So I need to add the T62_tn21 and an accompanying test in blom. Should I add any other resolution?

@mvertens - Thanks for the update, I will try your setup.

We often use T62_tn21 for testing, so that would be nice to have. We also use TL319_tn14 for OMIP-2 runs. In addition to T62_tn14, those are the main ones used for OMIP-type runs.

@mvertens
Copy link
Contributor Author

@TomasTorsvik - thanks so much for confirming this!
Moving forwards what do we want to set CPL_ALBAV for noresm? Do we want to adopt the same default as is used in CESM?

@TomasTorsvik
Copy link
Contributor

@TomasTorsvik - thanks so much for confirming this!
Moving forwards what do we want to set CPL_ALBAV for noresm? Do we want to adopt the same default as is used in CESM?

I think this may be a topic that we should discuss in a wider NorESM forum, although in this case it affects an OMIP-type run. My view for noresm development would be to use the same defaults as CESM, until at some point we need to tune the model,

@mvertens
Copy link
Contributor Author

@matsbn @TomasTorsvik @jmaerz - I was wondering if we could move forwards with merging this tag now given that we have a release branch. I have made progress with the tnx2v1 grid - but it turns out that cice6 does not support that resolution - so getting that in will be more involved than I had expected. As a result - I would like to keep that out of this PR.
I have generated our first set of baselines in
/cluster/shared/noresm/noresm_baselines/.

@mvertens
Copy link
Contributor Author

Just as another note - I need to create a new noresm v8 code base so that we can start new coupled model simulations and I'd like to use blom with the new namelist changes as part of that. I would also use this as a new step in migrating the new code from @jmaerz into the nuopc code.

@jmaerz
Copy link
Collaborator

jmaerz commented Aug 18, 2023

Dear @mvertens , could you elaborate a bit on what you mean with this migration step of the extended nitrogen cycle? Is there a chance to test, whether the current master of BLOM is still usable with the MCT coupler (anyone willing to do this?)? - if it is, I would suggest to merge the master into the feature-hamocc_beyond-CMIP6 branch (#268) to update that one with the new iHAMOCC-namelist option which I then could merge into the extended nitrogen cycle branch - and then eventually merge the extended nitrogen cycle branch into the feature-hamocc_beyond-CMIP6 branch (#269), which would make life easier and less spread out - also for the nuopc preparation @TomasTorsvik , @JorgSchwinger , @mvertens , what do you think about it? - I could prepare all this early next week.

@mvertens
Copy link
Contributor Author

@jmaerz - thanks for laying out a plan. I am confused as to when this PR could then get merged into master. Are you saying that you would like to have all of your PRs merged to master (i.e. #268 and #269) BEFORE we bring in this PR? Maybe a quick chat would be very helpful at this point. Would you have time this afternoon?

@jmaerz
Copy link
Collaborator

jmaerz commented Aug 18, 2023

I would have time, yes. However. I fear I am not exactly the right person to decide the steps. My point is that I would like to avoid double work with all the merging processes. My take from the last BLOM/NorESM development meeting with @matsbn was that with the new release branch (#267), we can break CMIP6 backwards compatibility when it comes to scientific results, however, being still able to run NorESM with the MCT coupler was some wish/requirement that @matsbn suggested (if I got it right - please correct me, if I am wrong). My comment is NOT affecting the merging of this PR into master, it was more directed to your comment about making the extended nitrogen cycle ready for nuopc, while still being able to do all the coupling as of NorESM as laid out in #147 (comment). I guess, I am still having issues to fully understand the consequences of your developments in BLOM on coupling abilities with different versions of NorESM...

As a brief information: thus far, from the iHAMOCC point of view, any developments in master aimed at scientific results backward compatibility with the CMIP6 version. Bug fixes that affected this backward compatibility were pushed into feature-hamocc_beyond-CMIP6 (or in short: beyond-CMIP6). We continuously merged developments done in master also into the beyond-CMIP6 branch to keep that up-to-date with master. My extended nitrogen cycle branch has these bug fixes from the beyond-CMIP6 branch and will eventually merged into the beyond-CMIP6 branch and bringing all this into master at some point. This does NOT affect your developments in master though (since these will be merged further into beyond-CMIP6). Care needs to be taken, once we introduce the #264 PR, since some of your code re-writings will potentially affect the bug fixes we did in the beyond-CMIP6 branch, but this is something we will be taking care of.

@mvertens
Copy link
Contributor Author

@jmaerz - thanks for your clarifications. I would definitely expect the new release branch to run with mct - but not necessarily the changes going into master (at least they would not be routinely tested). Sorry if my comment was confusing about the extended nitrogen cycle. I'd like to migrate the changes you needed to put into the mct cap and the mct coupler for your changes to the extended nitrogen cycle into the blom nuopc cap and cmeps. I would view a good starting point for that as being master once this PR is accepted.

@mvertens
Copy link
Contributor Author

I am also happy to defer the #264 PR until after we get your changes migrated into the NUOPC cap and CMEPS.

@matsbn
Copy link
Contributor

matsbn commented Aug 23, 2023

I tried this PR using NorESM2.0.6 (tag release-noresm2.0.6). I got the following error during case.setup:

  File "/cluster/projects/nn9560k/matsbn/GitHub/matsbn/NorESM2.0.6/components/blom//cime_config/buildnml", line 20, in <module>
    from CIME.Tools.standard_script_setup import *
ModuleNotFoundError: No module named 'CIME'

Other CIME modules seem to load fine using NorESM2.0.6. Do you think it is possible to overcome this issue @mvertens?

@mvertens
Copy link
Contributor Author

@matsbn - thanks for trying this out. Yes - I can find a way around this I think.

@mvertens
Copy link
Contributor Author

@matsbn - I'm pushing things back but its not quite working yet. I'll let you know when I have things working.

@matsbn
Copy link
Contributor

matsbn commented Aug 23, 2023

@matsbn - I'm pushing things back but its not quite working yet. I'll let you know when I have things working.

Great that you are having a look at this!

@mvertens
Copy link
Contributor Author

I have gotten it working in terms of the namelist generation and build - but for some reason I submit it to the queue and it starts up and then immediately dies. No log file or anything. I'd like to duplicate exactly what you did.
What create_newcase command did you use out of the release code (release-noresm2.0.6)?

@mvertens
Copy link
Contributor Author

@matsbn - I have pushed a fixes back that now enables buildnml work with the older version of CIME that is in the release code with blom replaced by this PR.
./create_newcase --case /cluster/home/mvertens/noresm/test_blom_newnl --compset NOINY --res T62_tn14 --project nn9560k --mach betzy
I also ran the original checkout and verified that everything was bit-for-bit.
Hopefully, you can confirm that this also works for you.

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.

I get bit-identical results when testing the NOIIAOC20TR compset in NorESM2.0.6 and ensuring that the number of CICE cores match. This is for both BLOM_VCOORD options (isopycnic, hybrid). The only difference is that bromoform is enabled by default compared to the BLOM master I compared against. Maybe that is intentional, but leave it to @TomasTorsvik , @JorgSchwinger and @jmaerz to comment on that.

@TomasTorsvik
Copy link
Contributor

@mvertens , @matsbn - thanks to both of you for testing this PR.
I will meet with Jöran and Jörg today, hopefully we can move forward with this.

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 think we can merge this PR now.
If everyone agrees, I can do the merging.

@TomasTorsvik TomasTorsvik self-assigned this Aug 30, 2023
@mvertens
Copy link
Contributor Author

@TomasTorsvik @jmaerz @Mats - thanks all for your careful review of this PR. I think that it will make new regression testing much easier moving forwards.

@TomasTorsvik TomasTorsvik merged commit b1456a7 into NorESMhub:master Aug 30, 2023
@mvertens mvertens deleted the feature/refactor_buildnml branch May 15, 2024 10:29
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
Development

Successfully merging this pull request may close these issues.

5 participants