Add CF-compliant Time coordinate #5202

xylar · 2022-09-25T04:59:45Z

This variable has CF-compliant units and calendar. To accommodate the CF-compliant units, a new config option has been added to supply the reference time, 0001-01-01 by default.

This merge also adds a Time_bnds variable. For output from a single point in time, the values in Time_bnds are the same as Time. For time-averaged (or min or max) output, Time_bnds contains the start and end times of the interval covered by the output.

See E3SM-Ocean-Discussion#19 for more discussion

[BFB]

This variable has CF-compliant units and calendar. To accommodate the CF-compliant units, a new config option has been added to supply the reference time, 0001-01-01 by default.

xylar · 2022-09-25T05:42:07Z

I'm adding in progress because this is failing test execution for some tests and baseline comparison for all tests in the compass pr test suite on Anvil. I need to figure out why.

xylar · 2022-09-25T06:08:51Z

Okay, I just pointed at the wrong baseline path. After fixing this, all tests pass. (Not sure why validation failures are showing up as execution failures for a few tests but it seems to have to do with depending on a cached step -- nothing to do with this PR in any case.)

xylar · 2022-09-25T06:09:38Z

I'll run some E3SM tests as well, including a 5-year QU240wLI test so I can make sure MPAS-Analysis still runs fine.

xylar · 2022-09-25T06:10:28Z

@mark-petersen and @cbegeman, let me know what kind of testing you'd like me to do to help with the review process.

xylar · 2022-09-25T06:18:55Z

I ran the QU240 version of the daily_output_test and verified that I see Time and Time_bnds as expected:

$ ncdump -h forward/analysis_members/mpaso.hist.am.timeSeriesStatsDaily.0001-01-01.nc 
netcdf mpaso.hist.am.timeSeriesStatsDaily.0001-01-01 {
dimensions:
	Time = UNLIMITED ; // (1 currently)
	bnds = 2 ;
	nCells = 7332 ;
	nVertLevels = 16 ;
	nEdges = 22968 ;
	nVertLevelsP1 = 17 ;
	StrLen = 64 ;
variables:
	double Time(Time) ;
		Time:bounds = "Time_bnds" ;
		Time:standard_name = "time" ;
		Time:long_name = "time" ;
		Time:units = "days since 0001-01-01 00:00:00" ;
		Time:calendar = "noleap" ;
	double Time_bnds(Time, bnds) ;
		Time_bnds:long_name = "time bounds" ;
...
data:

 Time = 0.5 ;

 Time_bnds =
  0, 1 ;
}

xylar · 2022-09-25T18:34:37Z

I am having trouble with a BFB test I was trying to run, as reported in #5204

I am in the process of running a 5-year QU240wLI run for MPAS-Analysis testing. In that simulation, I'm seeing:

$ ncdump -v Time,Time_bnds 20220925.GMPAS-IAF.T62_oQU240wLI.chrysalis.mpaso.hist.am.timeSeriesStatsMonthly.0001-01-01.nc | tail
		:config_rx1_min_levels = 3 ;
		:config_rx1_min_layer_thickness = 1.f ;
		:file_id = "3wxtme9rle" ;
data:

 Time = 15.58333 ;

 Time_bnds =
  0.1666667, 31 ;
}

the expected first value for Time_bnds should be 0 and Time should be 15.5. This seems to be something that happens in timeSeriesStatsMonthly each time there is a restart, and it needs to be fixed. I'm on the fence about whether to fix it before, during, or after this PR. This is not directly related to this PR and instead has been a long-standing problem.

xylar · 2022-09-25T19:06:38Z

In MPAS-Analysis, I'm seeing the following type of error for WOCE and SOSE transects:

analysis task woceTransects_remapTransects failed during run
Traceback (most recent call last):
  File "/gpfs/fs1/home/ac.xylar/mpas-work/MPAS-Analysis/develop/mpas_analysis/shared/analysis_task.py", line 322, in run
    self.run_task()
  File "/gpfs/fs1/home/ac.xylar/mpas-work/MPAS-Analysis/develop/mpas_analysis/ocean/compute_transects_subtask.py", line 274, in run_task
    self._vertical_interp(ds, transectIndex, dsObs,
  File "/gpfs/fs1/home/ac.xylar/mpas-work/MPAS-Analysis/develop/mpas_analysis/ocean/compute_transects_subtask.py", line 395, in _vertical_interp
    ds = ds.where(ds.transectNumber == transectIndex, drop=True)
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/common.py", line 1081, in where
    return ops.where_method(self, cond, other)
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/ops.py", line 177, in where_method
    return apply_ufunc(
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/computation.py", line 1192, in apply_ufunc
    return apply_dataset_vfunc(
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/computation.py", line 480, in apply_dataset_vfunc
    result_vars = apply_dict_of_variables_vfunc(
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/computation.py", line 422, in apply_dict_of_variables_vfunc
    result_vars[name] = func(*variable_args)
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/computation.py", line 771, in apply_variable_ufunc
    result_data = func(*input_data)
  File "/gpfs/fs1/home/ac.xylar/anvil/mambaforge/envs/mpas_dev/lib/python3.10/site-packages/xarray/core/duck_array_ops.py", line 275, in where_method
    other = dtypes.get_fill_value(data.dtype)
AttributeError: 'cftime._cftime.DatetimeNoLeap' object has no attribute 'dtype'

This suggests that there will, indeed, need to be some fixes to MPAS-Analysis to support the new time coordinate. These fixes will need to be coordinated so that they are available in a new E3SM-Unified release before this change goes in.

xylar · 2022-09-25T20:23:47Z

The MPAS-Analysis fix is here:
MPAS-Dev/MPAS-Analysis#904

I'm still testing it out but will update here.

cbegeman

Approving based on testing in E3SM-Ocean-Discussion#19.

@xylar Let me know if you'd like me to run any additional tests.

mark-petersen · 2022-09-26T20:06:21Z

Passes nightly suite with gnu debug and intel. gnu matches bfb with previous.

mark-petersen · 2022-09-26T20:29:00Z

Passes ./create_test SMS_D.T62_oQU120_ais20.MPAS_LISIO_TEST.chrysalis_gnu -q acme-small --walltime 30:00

mark-petersen · 2022-09-26T20:52:43Z

I tested here

/lustre/scratch5/turquoise/mpeterse/runs/n/ocean_model_220926_1523108c_ba_gfortran_openmp_cf_compliant/ocean/global_ocean/QU240/PHC/analysis_test/forward/analysis_members

with

725     config_AM_timeSeriesStatsDaily_enable = .true.

This works correctly for daily output. Here I use:

423 <stream name="timeSeriesStatsDailyOutput"
424         type="output"
425         filename_template="analysis_members/mpaso.hist.am.timeSeriesStatsDaily.$Y-$M-$D.nc"
426         filename_interval="00-01-00_00:00:00"
427         reference_time="01-01-01_00:00:00"
428         clobber_mode="truncate"
429         precision="single"
430         io_type="pnetcdf"
431         packages="timeSeriesStatsDailyAMPKG"
432         output_interval="00-00-02_00:00:00" >

output is

 Time_bnds =
  1, 2,
  3, 4,
  5, 6,
  7, 8,
  9, 10 ;
}

It appears to have trouble with 12-hourly frequency:

423 <stream name="timeSeriesStatsDailyOutput"
424         type="output"
425         filename_template="analysis_members/mpaso.hist.am.timeSeriesStatsDaily.$Y-$M-$D.nc"
426         filename_interval="00-01-00_00:00:00"
427         reference_time="01-01-01_00:00:00"
428         clobber_mode="truncate"
429         precision="single"
430         io_type="pnetcdf"
431         packages="timeSeriesStatsDailyAMPKG"
432         output_interval="00-00-00_12:00:00" >

there seems to be some integer rounding on the start bounds:

ncdump -v Time_bnds mpaso.hist.am.timeSeriesStatsDaily.0001-01-01.nc|tail -n 5
 Time_bnds =
  0, 1,
  1, 1.5,
  1, 2 ;
}

But maybe time_bounds is not mean to be sub-daily for climate data?

It would be nice to put units on the time_bounds, if allowed by the CF convention:

        float Time_bnds(Time, bnds) ;
                Time_bnds:long_name = "time bounds" ;

xylar · 2022-09-27T00:12:43Z

@mark-petersen, Time_bnds should work fine for subdaily. Could you verify that xtimeStart_Daily and xtimeEnd_Daily are correct in the subdaily example you have? If no, I suspect this is an issue with timeSeriesStats and not with the Time_bnds.

xylar · 2022-09-27T00:14:24Z

@mark-petersen, changing to output_interval="00-00-00_12:00:00" is not sufficient on its own. There are a couple of namelist options you need to change as well. I think having a file interval that is shorter than the computing interval is liable to cause weird behavior like this.

xylar · 2022-09-27T00:15:05Z

@cbegeman, thanks for approving

@xylar Let me know if you'd like me to run any additional tests.

No, what you already did is enough, I think.

mark-petersen

Approving by inspection and testing above. Thanks!

xylar · 2022-09-28T21:33:22Z

Thanks @mark-petersen!

xylar · 2022-09-28T23:39:11Z

I think we don't need to fix the issue in #5202 (comment) before merging. I will create a compass test case to investigate this and hopefully that will help us fix it.

xylar · 2022-09-28T23:40:32Z

The issue in MPAS-Analysis mentioned in #5202 (comment) has been fixed in v1.7.2. I plan to do an emergency release of E3SM-Unified soon that includes this fix.

xylar · 2022-09-30T17:14:56Z

I tested ERS.ne11_oQU240.WCYCL1850NS.anvil_intel and ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel with a test merge of master into this branch and results are BFB, as expected. (I needed to do a fresh checkout of master to resolve #5204.)

Add CF-compliant Time coordinate This variable has CF-compliant units and calendar. To accommodate the CF-compliant units, a new config option has been added to supply the reference time, 0001-01-01 by default. This merge also adds a Time_bnds variable. For output from a single point in time, the values in Time_bnds are the same as Time. For time-averaged (or min or max) output, Time_bnds contains the start and end times of the interval covered by the output. [BFB]

jonbob · 2022-10-06T22:34:24Z

passes:

ERS_Ld5.T62_oQU120.CMPASO-NYF.chrysalis_intel
SMS_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod
SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod

merged to next

jonbob · 2022-10-07T15:19:04Z

merged to master

xylar · 2022-10-07T15:58:58Z

Thanks so much, @jonbob! And to @mark-petersen and @cbegeman for the reviews! I'm excited to finally have this in E3SM!

Update bld files to catch missed mpas-o Registry change A previous PR, #5202, brought in a new config in the mpas-ocean Registry file, but the bld files were not updated to match. This brings the E3SM bld files up-to-date with the Registry [NML] [BFB]

xylar added 7 commits September 24, 2022 22:24

Add Time coordinate

a4f641c

This variable has CF-compliant units and calendar. To accommodate the CF-compliant units, a new config option has been added to supply the reference time, 0001-01-01 by default.

Add Time_bnds variable

cc931c2

Fix typos and white-space in time series stats

c178ffa

Clean up input and output field names

d3b80ae

Always add Time and Time_bnds to time series stats

31ffa23

Compute Time and Time_bnds in time series stats

4733d97

Add standard name

1523108

xylar added mpas-ocean BFB PR leaves answers BFB labels Sep 25, 2022

xylar requested review from mark-petersen and cbegeman September 25, 2022 04:59

xylar assigned jonbob Sep 25, 2022

xylar mentioned this pull request Sep 25, 2022

Add CF-compliant Time coordinate E3SM-Ocean-Discussion/E3SM#19

Closed

xylar marked this pull request as draft September 25, 2022 05:40

xylar changed the title ~~Add CF-compliant Time coordinate~~ [WIP] Add CF-compliant Time coordinate Sep 25, 2022

xylar added the in progress label Sep 25, 2022

xylar removed the in progress label Sep 25, 2022

xylar changed the title ~~[WIP] Add CF-compliant Time coordinate~~ Add CF-compliant Time coordinate Sep 25, 2022

xylar marked this pull request as ready for review September 25, 2022 06:07

xylar mentioned this pull request Sep 25, 2022

Test failing with "ERROR: Requested field initialCarbon not available" #5204

Closed

xylar mentioned this pull request Sep 25, 2022

Don't decode time as part of transect remapping MPAS-Dev/MPAS-Analysis#904

Merged

cbegeman approved these changes Sep 26, 2022

View reviewed changes

mark-petersen approved these changes Sep 28, 2022

View reviewed changes

This was referenced Oct 5, 2022

Start time in timeSeriesStats output from MPAS-Ocean not correct after restart #5217

Open

Fix two bugs in timeSeriesStats output in MPAS-Ocean #5218

Closed

Add TimeSeriesStatsRestartTest to global ocean MPAS-Dev/compass#435

Closed

jonbob merged commit 3cfcd85 into E3SM-Project:master Oct 7, 2022

xylar deleted the ocn/add-cf-time branch October 7, 2022 15:58

xylar mentioned this pull request Nov 19, 2022

Update E3SM-Project submodule MPAS-Dev/compass#461

Merged

32 tasks

jonbob mentioned this pull request Mar 21, 2023

Update bld files to catch missed mpas-o Registry change #5541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CF-compliant Time coordinate #5202

Add CF-compliant Time coordinate #5202

xylar commented Sep 25, 2022 •

edited by jonbob

Loading

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022 •

edited

Loading

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

cbegeman left a comment

mark-petersen commented Sep 26, 2022

mark-petersen commented Sep 26, 2022

mark-petersen commented Sep 26, 2022

xylar commented Sep 27, 2022

xylar commented Sep 27, 2022

xylar commented Sep 27, 2022

mark-petersen left a comment

xylar commented Sep 28, 2022

xylar commented Sep 28, 2022

xylar commented Sep 28, 2022

xylar commented Sep 30, 2022

jonbob commented Oct 6, 2022

jonbob commented Oct 7, 2022

xylar commented Oct 7, 2022

Add CF-compliant Time coordinate #5202

Add CF-compliant Time coordinate #5202

Conversation

xylar commented Sep 25, 2022 • edited by jonbob Loading

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022 • edited Loading

xylar commented Sep 25, 2022

xylar commented Sep 25, 2022

cbegeman left a comment

Choose a reason for hiding this comment

mark-petersen commented Sep 26, 2022

mark-petersen commented Sep 26, 2022

mark-petersen commented Sep 26, 2022

xylar commented Sep 27, 2022

xylar commented Sep 27, 2022

xylar commented Sep 27, 2022

mark-petersen left a comment

Choose a reason for hiding this comment

xylar commented Sep 28, 2022

xylar commented Sep 28, 2022

xylar commented Sep 28, 2022

xylar commented Sep 30, 2022

jonbob commented Oct 6, 2022

jonbob commented Oct 7, 2022

xylar commented Oct 7, 2022

xylar commented Sep 25, 2022 •

edited by jonbob

Loading

xylar commented Sep 25, 2022 •

edited

Loading