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

Aux cpl hist daily #378

Merged
merged 12 commits into from
May 15, 2023
Merged

Aux cpl hist daily #378

merged 12 commits into from
May 15, 2023

Conversation

kdraeder
Copy link
Contributor

@kdraeder kdraeder commented May 6, 2023

Description of changes

Attempt to write 'daily' auxiliary mediator history files when the forecast is shorter than a day.

Specific notes

It appears that I should be able to set all the relevant attributes
of the auxiliary mediator (was "coupler") history files using mediator/drv/cpl namelist variables.
The default values from cmeps/cime_config/namelist_definition_drv.xml are:

user_nl_cpl
  histaux_atm2med_file5_enabled = .false.
  histaux_atm2med_file5_history_option = 'ndays'
  histaux_atm2med_file5_history_n = 1
  histaux_atm2med_file5_ntperfile = 1
  histaux_atm2med_file5_auxname = 'atm.24h.avrg'

but changing these to

histaux_atm2med_file5_enabled = .true.
histaux_atm2med_file5_history_option = 'nhours'
histaux_atm2med_file5_history_n = 6

fails because 6 is an invalid value. Trying to make it be valid by
replacing the {ndays,1} in namelist_definition_drv.xml with {nhours,6) doesn't help.

./case.build --clean-all
doesn't help; ./case.build (preview-namelists) fails for the same reason.

I'm not able to trace through the abstractions in python
to where the comparison is made between the values from user_nl_cpl
and namelist_definition_drv.xml, so I'm not able to suggest a fix.

Contributors other than yourself, if any: none

CMEPS Issues Fixed (include github issue #):
Original issue was opened in CIME; ESMCI/cime#4409

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)
Auxiliary files with averaged fields, written every 6 hours, will have different contents
than those written once/day. But they should be correct if any averaging is done using the 6 hour span.

Any User Interface Changes (namelist or namelist defaults changes)?
The desired forecast span and frequency of output of the auxiliary mediator history data
should be settable in user_nl_cpl.
It appears that changes to cime_config/namelist_definition_drv.xml (and more?) are needed,
but I haven't figured out what those are.
Add valid_values attributes?

Testing performed

cmeps0.14.27-2-g0d2a0c8
in cesm2_3_alpha12e-1-g02ed0b8
An unmodified cmeps, run for 6 hours with

user_nl_cpl:
histaux_rof2med_file1_enabled = .true.
histaux_atm2med_file1_enabled = .true.
histaux_atm2med_file2_enabled = .true.
histaux_atm2med_file4_enabled = .true.
histaux_atm2med_file5_enabled = .true.

wrote out

   CESM2.2.1_multi_dflt.cpl_0001.hx.atm.3h.avrg.1979-01-01-10800.nc
   CESM2.2.1_multi_dflt.cpl_0001.hx.atm.1h.avrg.1979-01-01-03600.nc
   CESM2.2.1_multi_dflt.cpl_0001.hx.atm.1h.inst.1979-01-01-03600.nc

but failed to produce the rof.24h.avrg and atm.24h.avrg (file1) files.

Adding variables to user_nl_cpl to override the defaults didn't help:

histaux_rof2med_file1_history_option = nhours
histaux_rof2med_file1_history_n = 6
histaux_rof2med_file1_auxname = 'rof.24h.avrg'

Changing the defaults in namelist_definition_drv.xml also didn't help.

…ux_cpl_hist_daily

Because git told me to do it.
I'm trying to set up a remote to my new fork of CMEPS
in order to push changes on the aux_cpl_hist_daily branch.
@kdraeder
Copy link
Contributor Author

kdraeder commented May 6, 2023

A few minor items and questions from namelist_definition_drv.xml:

  1. Why are X_history_n and X_ntperfile type 'char', except for histaux_atm2med_file1_ntperfile, which is integer?
  2. Should 'sames' be 'samples'?
  3. It would be helpful if the X_flds had 'fields' in them instead of just being repetitions of the X_enabled .
  4. There's inconsistent use of 'file #' and 'file#' in the namelist_definition_drv.xml comments.
  5. histaux_atm2med_file[45]_flds don't look at all like precip fields, which are handled by file3.

I didn't want to clutter the PR with those until the main issue is resolved
and I know whether the minor things are worth fixing.

@billsacks
Copy link
Member

I want to make sure I'm understanding your goal correctly.

The PR comment makes it sound like your main goal is to be able to change these history file frequencies for your specific case and ran into problems doing so. But your changes alter the default frequencies for a few of the mediator history files. I can't speak to the importance of these frequencies for rof (Keith Lindsay could probably speak to that, but let's wait to pull him in to this discussion), but I know that the atm aux hist file frequencies have just been carefully vetted by Keith Oleson (again, let's wait to pull him into the discussion).

Is the main issue that you wanted to be able to change these for a specific case but ran into problems setting them in user_nl_cpl? If so, let's fix that problem. I have a feeling that it comes down to the data type issue you identified in your follow-up comment: it looks like these history_n variables are mistakenly listed as char when they should be integers, so the namelist generation is expecting a string ('6') rather than integer (6) in user_nl_cpl. Can you try backing out these changes then retrying your original user_nl_cpl changes but with <type>char</type> changed to <type>integer</type> for the relevant variables (e.g., histaux_atm2med_file5_history_n)? If that works, let's get that change in for all variables, along with fixing the other typos you noted in your other comment. Thank you for your careful documentation of those issues!

@kdraeder
Copy link
Contributor Author

kdraeder commented May 10, 2023

@billsacks thanks for the strategy.
The goal is to run a 6 hour forecast, as required for data assimilation, and have the 'daily' files
(file5 and the rof2med aux hist file = X below) written at the end. Later I concatenate a series of those
into a more standard-looking file.
I'm aware that there could be some issues with the averaging.

Overall, I think that this cmeps mechanism for history file output is a big improvement!
But it's not yet living up to its promised flexibility.
I first tried just adding X_enabled = .true. to the user_nl_cpl, but those 2 files did not appear.
3 other hist aux files that I requested did appear. They have _history_option = nhours.

I reverted all my previous changes to namelist_definition_drv.xml and changed the chars to integers,
but those 2 files don't show up.

I changed X__history_option = nhours and X__history_n = 6 in namelist_definition_drv.xml,
but they still didn't show up. X_ntperfile still = 1.
CORRECTION; the atm2med file did show up, but not the rof2med.

I changed histaux_atm2med_file5_history_n = 1 and then it was written out hourly (yay!).

So nhours can work, but something goes wrong with writing a single file at the end.

I'll want to either change the default X_history_option to nhours,
or make that an acceptable option, or make the 'daily' files be written at the ENDOFRUN.

@jedwards4b
Copy link
Collaborator

@kdraeder is this PR ready to merge or would you consider it a work in progress? If the later can you please convert it to a draft using the button at the top right of the web page. Can you also point me to the cesm2.3 case that you have been experimenting with? As I understand it the first issue is to generate the rof.24h.avrg and atm.24h.avrg (file1) files.

@kdraeder
Copy link
Contributor Author

It looks like the 'char' -> 'integer' fixes definitely need to be made,
but that's not the whole solution, and I haven't even pushed those,
so I'll change it to a draft PR.

Note the correction to my earlier comment; setting atm2med_file5_history_option = nhours and ..._n = 6
did yield a .hx. file, with -21600.nc in its name and one time of data at hour 3.
But the rof2med file still did not show up, despite the same settings.

Meanwhile, I tried setting X_history_n = 3, and X_ntperfile = 2 and again got a file for atm2med,
but it has only 1 time in it, at hour 1.5. I was hoping for 2
Still no rof2med.

@kdraeder kdraeder marked this pull request as draft May 10, 2023 20:29
@jedwards4b
Copy link
Collaborator

I've opened a PR to your branch kdraeder#1
and I'm happy to take on the other issues - but I'm having trouble parsing your comments.
Can you lay them out one at a time including the case you are running, the namelist changes you make,
the actual result and the expected result?

@kdraeder
Copy link
Contributor Author

Thanks @jedwards4b

The CESM clone is in /glade/work/raeder/Models/cesm_dflt_2023-4-20 (cesm2_3_alpha12e-1-g02ed0b8,
cmeps0.14.27-2-g0d2a0c8).

The caseroot is /glade/work/raeder/Exp/CESM2.2.1_aux_cpl (misleading name due to cloning misdirection).
The mediator aux hist files are turned on in user_nl_cpl, and I changed file names from .24h. to .6h.

The relevant test output is in /glade/scratch/raeder/CESM2.2.1_aux_cpl/run/N*
which have the .hx. files and the cmeps/cime_config/namelist_definition_drv.xml used to create them.
All of these namelists have the char -> integer fix for both history_n and history_ntperfile for the atm and rof aux med hist files.
All experiments are 6 hour forecasts. I hoped or expected that the "daily" files would be written at the end.
The N* directory names summarize the differences in history_option (atm == rof) and history_n for atm and rof separately.

EXPERIMENT history_option atm history_n atm file5 ("daily") rof history_n rof file1 ("daily")
Ndays_atm1_rof1 ndays(dflt) 1 (dflt) no 1 (dflt) no
Nhours_atm6_rof6 nhours 6 yes; t = 3 h 6 no
Nhours_atm3_rof3 ntperfile = 2 nhours 3 yes; t = 1.5 h only, expected 4.5 too 3 no

So the rof "daily" file was not written, no matter what I tried.
The atm "daily" file was written when history_option was nhours, but only one timeslot,
which is good enough if only 1 is requested, but not otherwise.

@jedwards4b
Copy link
Collaborator

@kdraeder I see a fundamental problem in med_io_mod.F90, it won't currently handle more than one component at a time. I will work on a fix today.

@jedwards4b jedwards4b mentioned this pull request May 11, 2023
@kdraeder
Copy link
Contributor Author

The missing "daily" files appear to have been fixed by https://github.com/jedwards4b/CMEPS/tree/history_n .
And the typo "sames" has been fixed.

The remaining unfixed items in cmeps/cime_config/namelist_definition_drv.xml from above are

  1. It would be helpful if the X_flds had 'fields' in them instead of just being repetitions of the X_enabled .
  2. There's inconsistent use of 'file #' and 'file#' in the namelist_definition_drv.xml comments.
  3. histaux_atm2med_file[45]_flds don't look at all like precip fields, which are handled by file3.

I have candidate fixes for those, if that would be useful.
Should I provide them as a PR?

@jedwards4b
Copy link
Collaborator

@kdraeder sure please provide the additional fixes.

@jedwards4b jedwards4b self-assigned this May 11, 2023
@jedwards4b
Copy link
Collaborator

Tests qcmd -- ./create_test --xml-testlist ../../components/cmeps/cime_config/testdefs/testlist_drv.xml --xml-machine cheyenne --xml-compiler intel
was run with this PR. All passed except ERR_Vnuopc_Ld5.f09_t061.B1850MOM.cheyenne_intel.allactive-defaultio
Which failed due to a ctsm issue.

@mvertens
Copy link
Collaborator

@jedwards4b - I really like your new implementation. Its much better than what I had originally put in place. Except for a very picky request for consistent indentation I think this is great!

@mvertens mvertens self-requested a review May 12, 2023 14:37
@DeniseWorthen
Copy link
Collaborator

For non-cesm use (since we don't make use of the aux functionality), does this change mediator history file use at all?

@jedwards4b
Copy link
Collaborator

@DeniseWorthen it should not change functionality of mediator history file use at all.

mediator/med_phases_history_mod.F90 Outdated Show resolved Hide resolved
@jedwards4b jedwards4b marked this pull request as ready for review May 12, 2023 22:10
Copy link
Collaborator

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

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

Everything appears to be working as expected for UFS

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.

None yet

5 participants