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

cam-fv perfect_model_obs for CESM2_1 #233

Closed
wants to merge 6 commits into from
Closed

cam-fv perfect_model_obs for CESM2_1 #233

wants to merge 6 commits into from

Conversation

kdraeder
Copy link
Contributor

@kdraeder kdraeder commented May 28, 2021

Description:

There are no perfect_model scripts in cam-fv/shell_scripts/cesm2_1.
Some existing scripts which will interact with pmo scripts need updating to do that.

Fixes issue

#225

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
    The last commit message lists the changed scripts and summarizes the changes.

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

The perfect model scripts in this PR have been used to set up and run a multicycle run
to harvest observations from CESM2_1:CAM6.
The test used /glade/p/cisl/dares/Observations/Synthetic/UVTRadiosonde_3456
Other details of the test are embedded in
/glade/work/raeder/Exp/Zagar_OSSE_pmo/setup_pmo.original

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Version tag

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

kdraeder added 2 commits May 5, 2021 17:47
They're untested.

setup_pmo
>  Modeled after setup_hybrid
>  May not use perfect_model.csh.template yet

perfect_model.csh.template
>  Modeled after assimilate.csh.template

setup_hybrid
>  Minor improvements stemming from creating setup_pmo

assimilate.csh.template
>  Minor improvements stemming from creating perfect_model.csh.template
It prompted minor changes in other cam-fv scripts.
Has passed a multiple cycle test using
Observations/Synthetic/UVTRadiosonde_3456.
Documentation has not been updated.

models/cam-fv/shell_scripts/cesm2_1/setup_pmo
>  Script to set up a perfect_model case.  Modeled after setup_advanced.
>  (But it has hi-res SST option, like setup_advanced).

models/cam-fv/shell_scripts/cesm2_1/perfect_model.csh.template
>  Template to be transformed by DART_config into the script
>  called by CESM to harvest the perfect obs.
>  Modeled after assimilate.csh.template.

models/cam-fv/shell_scripts/cesm2_1/DART_config.template
>  Updated to handle both assimilate.csh and perfect_model.csh.
>  Added a local variable DART_SCRIPT to be filled by setup_*
>  with the script name appropriate for this job

models/cam-fv/shell_scripts/cesm2_1/assimilate.csh.template
>  Better, comments, placement of a few commands, and prints.

models/cam-fv/shell_scripts/cesm2_1/setup_advanced,
   setup_hybrid,
   setup_advanced_Rean_2017
>  Added substitution of your_dart_script to the transformation
>  of DART_config.template to accommodate the perfect_model upgrade.

models/cam-fv/work/input.nml
>  Better name for the perfect_model output state file.
>  This is what's expecte4d by perfect_model.csh.
@kdraeder kdraeder added Documentation Improvements or additions to documentation Enhancement New feature or request Refactor Working but needs cleanup labels May 28, 2021
@kdraeder kdraeder self-assigned this May 28, 2021
@hkershaw-brown
Copy link
Member

Hi Kevin,

A couple of questions to clarify what we are reviewing here:

  • Is the updated documentation coming soon? Do you want reviewers to work though documentation and see if
    we can run the scripts successfully following the documentation.

  • The changes are for PMO. Are The DART_config.template, assimilate.csh.template, setup_advanced also used for filter?
    Do we need to test an assimilation cycle when changes are made to these scripts?

This is just a question for my own understanding ( not something to change for this pull request). The DART CESM scripts, e.g. assimilate.csh.template don't use ./xmlquery to find out machine-specific options, but do use xmlquery for other CESM options. Is this because DART will override some of the machine specific options, or xmlquery does give enough info, or something else?

@hkershaw-brown
Copy link
Member

one more question, the latest release of CESM is 2.2.1. Do we need to worry about that?

@kdraeder
Copy link
Contributor Author

kdraeder commented Jun 2, 2021

I wasn't planning to update documentation until we've settled on the code that I would document.
But if we want it to be a standard practice to do it all at once, and update both code and documentation
as the PR evolves, I'll make some time for that.

The DART_config.template, assimilate.csh.template, setup_advanced are used for filter,
so those should be tested as part of this. I believe that I did not break those code paths,
but I'll need to test it. I have a convenient opportunity to do that in the next day or 2.

Regarding xmlquery, the general answer is probably that we generally only import
the env variables that we need into each script, and many of them don't need machine
specific variables. We could view job submission variables as machine specific,
in which case assimilate.csh gets them directly from the run environment ($PBS_...).
The setup_ scripts do need some machine specific variables, but those are set by
the user at that point, and are then built into the CESM case and then can be queried
using xmlquery.
If you remember where you saw any, that could make the answer more specific.

At some level we need to worry about keeping up with CESM's releases,
but for the time being we need to be selective about which releases need our focus.
That's usually driven by whether a release has capabilities that a user needs.
The recommendation that came with the 2.2.0 release was to use 2.1.z for science
projects, but 2.2 for model development work, if it needed the new capabilities.

I had a near death experience trying to get enough changes into the CESM
development and testing workflow to ensure that each new release was automatically
compatible with DART. I had to give up the effort when I uncovered some inconsistencies
in coupler auxiliary history file names versus contents (ESMCI #2831),
which they were not willing to change in the time frame that I needed them,
which was ASAP so that I could run the Reanalysis.
I'd still like to do that, but it's a back burner idea.


# CESM uses C indexing on loops; cycle = [0,....,$DATA_ASSIMILATION_CYCLES - 1]
# "Fix" that here, so the rest of the script isn't confusing.

@ cycle = $2 + 1

cd ${CASEROOT}

setenv scomp `./xmlquery COMP_ATM --value`
Copy link
Member

Choose a reason for hiding this comment

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

note for xmlquery

This bit of the script uses xmlquery

@@ -98,6 +91,15 @@ else if ($?LSB_HOSTS) then

endif

if ($?JOBNAME) then
Copy link
Member

Choose a reason for hiding this comment

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

note for xmlquery (not critical for this review)
this bit (and above) is the machine specific stuff that does not use xmlquery

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the sort of variable I mentioned that comes directly from the job environment,
instead of from the CESM environment.
The COMP_ATM variable is not machine specific. It describes the atmospheric component
that's specified in the CESM component set.

@@ -71,7 +71,8 @@ echo "==================="

setenv DARTROOT your_dart_path
setenv DART_SCRIPTS_DIR $DARTROOT/models/cam-fv/shell_scripts/your_setup_script_dir
echo "DART_SCRIPTS_DIR = $DART_SCRIPTS_DIR"
setenv DART_SCRIPT your_dart_script
Copy link
Member

Choose a reason for hiding this comment

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

For the 'your_dart_script' variable. Is this ever going to be something other than 'assimilate.csh' or 'no_assimilate.csh'? I'm missing what the added flexibility here is for, would someone have variations on assimilate.csh?

sed -e "s#your_dart_script#assimilate.csh#"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I generalized this because perfect_model.csh is now one of the possibilities.
I can imagine people using variations of assimilate.csh, but that wasn't my goal.

Copy link
Member

Choose a reason for hiding this comment

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

oh ok, so the flexibility of 'your_dart_script' is for assimilate.csh or perfect_model.csh

@timhoar
Copy link
Contributor

timhoar commented Jun 7, 2021

How much similarity do we want to have between setup_pmo, setup_hybrid, and setup_advanced?
I'm not asking to go crazy, but there are a lot of nuisance differences between them. I think it is a good thing to have them differ where they need to be different - and be similar (if not identical) where it is reasonable. Once a person understands how to setup pmo, they can diff the scripts to get a feel for what has to change to set up a hybrid (assimilation) run.

I'm happy to take a run at reducing the nuisance differences and then leaving it to Kevin to see if there is anything more he'd like to borrow from one script for the others ... (like when/where to chmod 755 the DART_config)

--------------
All:
   Better comments.
   Improvements from more testing.
*  This version is untested.

--------------
models/cam-fv/shell_scripts/cesm2_1/

compress.csh
   Handle previously (de)compressed files.

DART_config.template
   Replaced num_instances with CESM's NINST
   Fixed DART_SCRIPTS_DIR directory redundancy.
   Define CIMEROOT env variable with value from setup_ to enable
     xmlchange.
   Substitute job characteristics into directives in *compress.csh
     and launch_cf.sh.
   Updated 'next steps' instructions.

assimilate.csh.template
   Make save_rest_freq handle day of the week (and every Nth day).
   Better purging of rh and h0 and stages_except_output files.
   Reduce unneeded listings of files, to save time.
   More selective hiding of files from the purging process.
   Only compress cpl history files if they exist.

setup_*
   Made more consistent with each other, including CAM external forcing,
     which matches what was used in the Reanalysis.
   Sed more case descriptors into DART_config
   Added warning about absence of inflation restart file to
     stage_cesm_files (since they are restarts).
     ? Maybe that should be done in DART_config.
setup_advanced
   Updated high res. SST file to the 2011-2020 version.
   Added some error numbers to exits.
   Added fix of docn.stream time labels after preview_namelists
     creates them.
setup_hybrid
   Updated for (1 degree) CESM2_1.
   Added (missing) optional call to DART_config.
setup_pmo
   Consolidated MultiInstanceRefcase into TRUTHinstance.
   Removed high resolution SST option (excessive for OSSEs).

--------------
models/cam-fv/work/input.nml
   Replaced stage preassim with forecast, to provide uninflated members
     (the means are identical).
   Replaced inflation flavor with enhanced adaptive (5).
   Imported improvement to hlevel_edges to reflect no GPS obs below 200m
     which improves plot appearance.
   Changed several variables to values most often used in CAM assims.
setup_pmo
setup_hybrid
setup_advanced
   Updated resolution examples, and web site to find more.
   Replaced generic cesm2_1 cesmtag with one that has a SourceMods.
   Replaced 1 degree, monthly, SST file with one that extends through
      2020 (actually part of 2021).
   Check whether start_year is withing the SST data file years.
   Print the git branch.
   More consistency between these 3 scripts.

submit_compress.csh
   Compress.csh is usually called from assimilate.csh,
   but sometimes it's run as a separate job.

launch_cf.sh
   Script to run shell script commands in parallel using
   the "command file" mechanism.
   Used by compress.csh
@kdraeder
Copy link
Contributor Author

kdraeder commented Aug 9, 2021

I've made the setup_{pmo,hybrid,advanced} scripts more consistent (@timhoar suggestion)
and tested all three updated scripts for a cycle (OK, a "day or two" turned into a month or 3).
I have not updated documentation, pending guidance from @hkershaw-brown about the best practice.

@hkershaw-brown can say whether I've answered her questions about the various sources of script variables;

  • the CESM environment (mostly from xmlquery),
  • the job environment ($PBS_ ...),
  • defined by the setup_* scripts.

One thing that may need to be fixed before this is acceptable is the question about
which CESM+CIME+SourceMods should be used by these scripts, and how users should get them.

@hkershaw-brown
Copy link
Member

Notes:

https://dart-documentation--233.org.readthedocs.build/en/233/models/CESM/doc/setup_guidelines.html warning on this page "developed in the context of DART’s Lanai release"

Five setup_ scripts.

setup_advanced
setup_advanced_Rean_2017
setup_hybrid
setup_pmo
setup_single_from_ens

@kdraeder
Copy link
Contributor Author

Notes:

https://dart-documentation--233.org.readthedocs.build/en/233/models/CESM/doc/setup_guidelines.html warning on this page "developed in the context of DART’s Lanai release"

Five setup_ scripts.

setup_advanced
setup_advanced_Rean_2017
setup_hybrid
setup_pmo
setup_single_from_ens

I haven't updated the documentation in this branch, so the reference to Lanai will (hopefully) disappear.
I see in https://docs.dart.ucar.edu/en/latest/models/CESM that there is no docs or docs/setup_guidelines.html.

My "setup_*" was misleadint.
Setup_advanced_Rean_2017 snuck in there via some earlier git usage mistake,
so it should probably be removed.
Setup_single_from_ens has almost nothing to do with assimilation
and was unaffected by the pmo script updates. It's used to spin up a single model state
from an available date to a useful date.

@hkershaw-brown
Copy link
Member

hkershaw-brown commented Aug 17, 2021

quick question, cesm2_1_relsd_m5.6 does not seem to exist as a tag or branch. I'm guessing I should use one of these tags:

release-cesm2.1.0
release-cesm2.1.1
release-cesm2.1.2
release-cesm2.1.3

@kdraeder
Copy link
Contributor Author

Yes, the cesm version is a question I'm grappling with.
In the short term you can use
/glade/work/raeder/Models/cesm2_1_relsd_m5.6
which is the version named in the scripts and is a combination of the released cesm2.1.0
and the cime that's available from github.com/kdraeder.
Some of the changes in the kdraeder cime were only needed for the Reanalysis.
For people without access to glade, that's somewhat cumbersome,
but I haven't figured out how necessary it is.

You can also grab SourceMods from ~raeder/cesm2_1_relsd_m5.6/SourceMods.

Ben has pointed out that the only supported version in that list is cesm2.1.3,
so I've been leaning toward adapting the scripts to that version
and hoping that the cime that comes with it works OK,
at least with modifications we can include in the SourceMods.
Which is another issue about this process; how to make those easily accessible to users.
But cesm2.2 was released (for development work only) in Sep 2020,
so I'm expecting a "scientifically validated" release soon, which would be a better target
for updating the scripts. But I also fear the next release; it may have a lot of NUOPC refactoring,
which Mariana has described as making it much easier to use CESM, but much harder
to understand (and modify) it, which we seem to always need to do.

# This script will delete an existing caseroot with the same name,
# so this script and other things you want to preserve should be kept elsewhere.
# dartroot Location of the root of _your_ DART installation.
# cime_output $cime_root/$CASEROOT is the directory where CESM's build and run directories will be created.
Copy link
Member

Choose a reason for hiding this comment

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

$cime_root/$CASEROOT does not match line 209

setenv cime_output /glade/scratch/${USER}

Copy link
Contributor Author

@kdraeder kdraeder Aug 19, 2021

Choose a reason for hiding this comment

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

Woops, that description is from an earlier version, which had the additional variable.
The comment should be

# cime_output $cime_output/$USER/$CASEROOT is the directory where CESM's build and run directories will be created.

'File status unknown' can be ignored.
'ERROR: cice.buildlib failed' can be ignored, unless you've changed the CICE code

2) cd ${RUNDIR}
Copy link
Member

Choose a reason for hiding this comment

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

${RUNDIR} does not seem to print the correct value:

Here is the output

CESM settings which are of special interest:
 
        CONTINUE_RUN: FALSE
        RESUBMIT_SETS_CONTINUE_RUN: TRUE
        RESUBMIT: 0
        RESUBMIT_SETS_CONTINUE_RUN: TRUE
        RUN_REFCASE: cesm2_1_relsd_m5.6
        RUN_REFDATE: 2016-12-10
        RUN_REFDIR: /glade/work/raeder/Models/CAM_init/FV1deg_cesm2_1/2016-12-10-00000
        RUN_REFTOD: 00000
        RUN_STARTDATE: 2009-08-02
        STOP_OPTION: nhours
        STOP_N: 6
        SSTICE_GRID_FILENAME: /glade/p/cesm/cseg/inputdata/share/domains/domain.ocn.fv0.9x1.25_gx1v7.151020.nc
        SSTICE_YEAR_ALIGN: 1850
        SSTICE_YEAR_END: 2021
        SSTICE_YEAR_START: 1850
        SSTICE_DATA_FILENAME: /glade/p/cesm/cseg/inputdata/atm/cam/sst/sst_HadOIBl_bc_0.9x1.25_1850_2020_c210521.nc
        SSTICE_STREAM: CAMDATA
        DOUT_S: FALSE
        DOUT_S_SAVE_INTERIM_RESTART_FILES: FALSE
        DOUT_S_ROOT: /glade/scratch/hkershaw/Kevin_test/Test0/archive
        RUNDIR: /glade/scratch/hkershaw/Kevin_test/Test0/run
        MPI_RUN_COMMAND: UNSET
        AVGHIST_DATE: -999
        AVGHIST_N: -999
        AVGHIST_OPTION: never
        DATA_ASSIMILATION: ['CPL:FALSE', 'ATM:FALSE', 'LND:FALSE', 'ICE:FALSE', 'OCN:FALSE', 'ROF:FALSE', 'GLC:FALSE', 'WAV:FALSE']
        DATA_ASSIMILATION_CYCLES: 1
        DATA_ASSIMILATION_SCRIPT: 
 

-------------------------------------------------------------------------
Time to check the case.

1)  Scan the output from this setup script for errors and warnings:
    ERROR, WARNING, 'No such file' (except for MOSART)
    'File status unknown' can be ignored.
    'ERROR: cice.buildlib failed' can be ignored, unless you've changed the CICE code

2)  cd /glade/scratch/hkershaw/Test0/run
    Check the files that were staged; follow the links to confirm the data sources.
    Check the compatibility between them and the namelists and pointer files.

3)  cd /glade/work/hkershaw/Exp/Test0
    Verify the CESM XML settings, especially in env_batch.xml and env_run.xml.
    ./xmlquery --partial <partial_string_of_interest> 
    is particularly useful.
    Check the contents of the user_nl_{component}* files (e.g. do CAM's external 
    forcing files cover your time period)

4)  The default initial configuration is to do no harvesting of observations.
    as defined by the 'no_assimilate.csh' in the env_run.xml file.
    When you are ready to harvest, change "no_assimilate" to "perfect_model"
    and follow the instructions in 
    /glade/work/hkershaw/Exp/Test0/DART_config. 

The rundir from cesm is /glade/scratch/hkershaw/Kevin_test/Test0/run not /glade/scratch/hkershaw/Test0/run

Copy link
Contributor Author

@kdraeder kdraeder Aug 19, 2021

Choose a reason for hiding this comment

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

Good catch. RUNDIR in setup_pmo is set (to CESM's default value)
before CESM's CIME_OUTPUT_ROOT is set to the value that you want (with Kevin_test in the name),
which the changes RUNDIR in CESM, but not in setup_pmo.

So the 'setenv RUNDIR' line should be moved to after the './xmlchange CIME_OUTPUT_ROOT' line:

./xmlchange CIME_OUTPUT_ROOT=${cime_output}
# Changing CIME_OUTPUT_ROOT changes RUNDIR in CESM, so change it here too.
setenv RUNDIR ./xmlquery RUNDIR --value

I'll commit that change (and in setup_hybrid and setup_advanced).

# If you have modifications to CESM, they should be provided in a
# CESM-structured SourceMods directory, which this script expects to be in
# $user/$cesmtag/SourceMods.
# sourcemods DART does not require a SourceMods directory in order to work with CESM.
Copy link
Member

Choose a reason for hiding this comment

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

Is this true?

# sourcemods    DART does not require a SourceMods directory in order to work with CESM.

I thought we did need sourcemods?

@hkershaw-brown
Copy link
Member

hkershaw-brown commented Aug 18, 2021

Hi Kevin,

I've only looked at the setup_pmo script.
I do find it difficult to tell what variables I should or should not be changing and how the scripts interact with each other. I'm not sure whether the changes in this request are for public consumption because they are not for a released version of CESM. So possibly that is not that important for this pull request.

However, I think at some point the scripts could be made more user friendly. For example, I think it is only ~5/10 things a user needs to specify. Can these go in a script that gets sourced at the start of any setup_script?

  • location of DART
  • cesm case directory
  • account number
  • Experiment directory
  • location of sourcemods
# setup_X
source user_options.csh 

do DART/CESM setup magic

I might be missing how much a user needs to edit the contents of setup_pmo or other setup scripts.

For SourceMods, have the source mods in cesm_X_X scripts directory, and have the setup script copy the SourceMods to the correct location before?

@kdraeder
Copy link
Contributor Author

In models/cam-fv/shell_scripts/cesm2_1/setup_hybrid:

@@ -160,10 +165,10 @@ setenv compset_args "${compset_args} --run-unsupported"
-# sourcemods DART no longer requires a SourceMods directory in order to work with CESM.
-# If you have modifications to CESM, they should be provided in a
-# CESM-structured SourceMods directory, which this script expects to be in
-# $user/$cesmtag/SourceMods.
+# sourcemods DART does not require a SourceMods directory in order to work with CESM.

Is this true?
I thought we did need sourcemods?

The "-" lines are what I intended to be in the script.
They were true when I started the Reanalysis project 2 years ago.
I ended up with SourceMods for cesm2.1.0 because of bug fixes
for CESM that became available after that, and to modify CESM to handle
the Reanalysis better. I haven't gone back verify that it will all run
with no SourceMods (but maybe suboptimally).
I suppose that now would be the time to do that.
And I suppose the comment should be changed to represent this particular CESM,
which should have some SourceMods, even if they're not required by DART.

For SourceMods, have the source mods in cesm_X_X scripts directory,
and have the setup script copy the SourceMods to the correct location before?

We've been keeping SourceMods separate from DART in an effort to follow the principle
that DART won't contain model code if it's maintained elsewhere (or something like that).
That's not necessarily the best way forward,
and it would be very convenient to keep SourceMods in the scripts directory,
or maybe in models/cam-fv/SourceMods.

I do find it difficult to tell what variables I should or should not be changing
and how the scripts interact with each other.

The set of variables that need to be changed depends on the user's environment and goal.
If they're on cheyenne and are doing a "standard" type of assimilation, then not many.
If they're on a non-NCAR machine and doing something less common, then lots of changes
are required.

I'm not sure whether the changes in this request are for public consumption
because they are not for a released version of CESM.
So possibly that is not that imported for this pull request.

CESM2.1.0 is a released version, but it's not "scientifically supported",
which means that it has not been used for long (coupled) climate runs,
which verify against the chosen data sets (obs and old model runs).
It's also not technically supported by SEWG because there are later
incremental updates, which don't change the fundamental model,
but expand capabilities, and may fix some bugs, often in components that CAM+DART
doesn't care about. SEWG actively supports the latest of those,
but the others are available.
We haven't been able to keep up with their release schedule,
partly because in the past the releases usually broke the DART interface
and it took a lot of work to track down how and fix it.

However, I think at some point the scripts could be made more user friendly.
For example, I think it is only ~5/10 things a user needs to specify.

I think that may not be true for the initial setup on their machine
and their experiment. After the initial setup, a series of experiments
might require just a few changes for each one.
I agree that they could be made more user friendly,
which would require a discussion about how friendly they should be,
versus flexible and maintainable and other characteristics.

Can these go in a script that gets sourced at the start of any setup_script?

 location of DART
 cesm case directory
 account number
 Experiment directory
 location of sourcemods

# setup_X
source user_options.csh

do DART/CESM setup magic

I might be missing how much a user needs to edit the contents of setup_pmo or other setup scripts.

I actually did something like that for the Reanalysis experiment (I'm glad that we were thinking
like you are!), but (so far) decided against it in the generic code since it seemed less necessary
andor useful in the experimental mode for which I expect these scripts to be used most.
It was valuable in the production mode by providing consistency among several scripts.
You can check out /glade/work/raeder/Exp/f.e21.FHIST_BGC.f09_025.CAM6assim.011/data_scripts.csh
In that case, though, I actually put code in the setup script to generate the data_scripts.csh
based on values the user provided in the setup script. See
/glade/u/home/raeder/DART/reanalysis_git/models/cam-fv/shell_scripts/cesm2_1/setup_advanced.
So the philosophy there was "set up the experiment by filling in values in setup_xxx"
instead of "set it up by providing values in a tiny script with little context,
which will be fed to a script you don't need to look into."

@nancycollins
Copy link
Collaborator

nancycollins commented Aug 19, 2021

i haven't followed all the details here - but i do have a couple comments.

  1. why is this being committed to the repo if it's specifically for an unavailable tag version? is it for a particular user, or ? if it's for users who want to try their own cam assimilations, then shouldn't the scripts be tested with a released and available version that we think users will get valid results from if they run an assimilation. sorry if you've already discussed the purpose here and i've missed it.

  2. just because you source a small config file doesn't preclude users needing to change other things in the script but it's still useful. you can set the rundir, the experiment name, etc - things that are common to PMO, filter, the diagnostic scripts, etc. in a single place. if each script sources the same file, you only have to make changes there once and don't run the risk of forgetting to change the same variable in multiple files. if users need to go into the script and change more, that's step 2 and you can document it as such. you've found it useful in your reanalysis work - i'd think it would be just as useful here.

edit: don't use number sign 2 without escaping it, which i forget how to do.

@kdraeder
Copy link
Contributor Author

kdraeder commented Aug 26, 2021

Based on the discussion here and in the software standup of 2021-8-19
I think that we need to make a plan with managably sized pieces.
The decisions were to:

  1. Discuss later which released CESMs to use; functional or software supported or scientifically supported.
  2. Change the setup philosophy from single script/job to "resources" file + setup scripts?
  3. Put CESM SourceMods into DART.
  4. Unanswered question; should I update external documentation simultaneously with the code?

Here's my rough description of the effort that will be required to implement those decisions.
I think that in the name of incremental development and committing useful code that works, this PR and issue should be completed using CESM version and DART code that already exists, and further discussion and development should be done in new issues.
One we've decided on the overall strategy, I'll work on answering whichever questions remain in this PR.

  1. Deciding which CESM releases are worthy of a DART interface probably won't have a clear answer. It depends on user needs and the state of evolution of CESM, which is unpredictable. After a choice is made it will be an unknown amount of work to figure out how CESM might have broken the interface and fix it. This will be even harder than in the past because of CESM's migration to NUOPC.

  2. The set of script variables a user needs to change will vary from a few to many, depending on their experiment and machine. We'll need to decide which should be in a resources file and which will be left in the setup scripts. The cutoff will almost certainly be arbitrary, so we'll need to document which are set where. Speaking of documentation, roughly a third of the lines in the setup scripts are documentation of the variables. Will that documentation migrate with the variables into the resource file, making it potentially hundreds of lines? Or will it be left in the setup scripts where the variables are used, which would make the resource file small but only understandable by looking at some documentation at the same time? In the end we'll be asking all users to interact with the resources file, and many to interact with the setup script too, and possibly not being able to provide thorough guidance about which is necessary. The style of resources file we're discussing here differs from what I used in the Reanalysis. There the resources file was created by the setup script, using values defined in the setup script.

  3. This one is relatively easy, but there are questions about which parts of CESM (and CIME) can be fixed for DART using SourceMods, and how to apply all the kinds of fixes.

  4. Documentation of the code outside of the code as it evolves through issues and pull requests will result in needing to rewrite recently written documentation and possibly slowing the code development. External documention after the code is set would be less (re)writing, but doesn't provide the reviewers with as much information, and brings in the risk of delaying the documentation. Will there be a general rule about this, or decided on a case-by-case basis?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Enhancement New feature or request Refactor Working but needs cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants