Jul/2023 - Usability Testing Plan for v0.6.0rc1

[tomvothecoder]

Goal Dates

• v0.6.0 testing completion: Start of Sep/2023
• v0.6.0 release date: Mid Sep/2023

Overview

This page discusses the usability testing plan for v0.6.0rc1. It includes a list of notable changes, the related APIs to tests using real-world datasets, and the assigned tester(s) for each test case. We have unit tests for all public APIs but they use generated dummy datasets, which might not capture some edge-cases found in real-world datasets.

Goals of testing:

1. Ensure v0.6.0 is robust and stable enough for release
2. Resolve any bugs
3. Handle edge cases that crop up
4. Improve usability of APIs where possible and if it makes sense (high value, good practices, not overly complex)

Installing v0.6.0rc1 for testing

Release candidates are published under the “xcdat_dev” channel in conda-forge.

Command: <conda|mamba> install -c conda-forge/label/xcdat_dev xcdat=0.6.0rc1

Changelog and Test Cases

View the full changelog here.

Features

PR: Functions to produce accurate time bounds by @pochedls in #418

Affected APIs:

1. xcdat.open_dataset()
   • add_bounds parameter changed from boolean to list of strings representing CF axes
   • add_bounds default value changed from add_bounds=True to add_bounds=["X","Y"]
   • add_bounds calls add_missing_bounds()
2. xcdat.open_mfdataset()
   • add_bounds parameter changed from boolean to list of strings representing CF axes
   • add_bounds default value changed from add_bounds=True to add_bounds=["X","Y"]
   • add_bounds calls add_missing_bounds()
3. Dataset.bounds.add_missing_bounds()
   • "X", "Y", and "Z" axes -- add bounds using coordinates as midpoints (calls add_bounds())
   • "T" axis -- add bounds using frequency of time coordinates (calls add_time_bounds())
4. Dataset.bounds.add_bounds()
   • Add bounds using coordinates as midpoints
   • Supports "X", "Y", "Z", and "T" axes
5. Dataset.bounds.add_time_bounds()
   • Supports "T" axis
   • method="freq" - Add bounds using frequency of time coordinates
   • method="midpoint - Add bounds using time coordinates as midpoints (same as ds.bounds.add_bounds(axis="T"))

PR: Update create_grid args to improve usability by @jasonb5 in #507

Affected APIs:

1. xcdat.create_grid
   • kwargs parameters are deprecated with x, y, and z parameters
   • x, y, and z parameters accept a single DataArray representing coordinates or a Tuple of DataArray representing coordinates and coordinate bounds (optional)
2. xcdat.create_axis
   • A new utility function for an creating axis, which can be used as a create_grid argument
   • Returns a tuple of coordinates and optional bounds (can be None)

PR: Add API extending xgcm vertical regridding by @jasonb5 in #388

Affected APIs:

1. Dataset.regridder.regrid

Bug Fixes

PR: Fix multi-file dataset spatial average orientation and weights when lon bounds span prime meridian by @pochedls in #495

Affected APIs:

1. Dataset.spatial.average()

[pochedls]

@tomvothecoder – I was trying this out via: mamba create -n xcdatrc -c conda-forge/label/xcdat_dev xcdat and I got:

nothing provides cf_xarray needed by xcdat-0.6.0rc1-pyhd1e7374_0

[tomvothecoder]

Thanks for reporting your output. I believe that command worked for me on acme1 (Linux). I'll look into it today.

[tomvothecoder]

I'll test the command with mamba and conda.

[tomvothecoder]

<conda|mamba> create -n xcdatrc -c conda-forge/label/xcdat_dev xcdat worked for me on Linux.

Maybe clearing your package cache using mamba clean might help fetch cf_xarray.

[pochedls]

I think you must have specified -c conda-forge as a preferred channel somewhere. This failed on my mac + linux. This worked:

conda create -n xcdatrc -c conda-forge -c conda-forge/label/xcdat_dev xcdat

[tomvothecoder]

I think I set strict channel priority and conda-forge is at the top, which is probably equivalent to specifying -c conda-forge explicitly with that command.

[jypeter]

Yet another loosely related comment (sorry!)

@pochedls I also use mamba (instead of conda) all the time, but I have found out about libmamba-solver this morning, which seems to basically add the mamba solver to conda.

I'd be interested to know if you get the same result as above with conda --solver=libmamba, if you have time to give it a try.

It should be the same, of course...

Worth reading: conda Roll-out plan 2023

[tomvothecoder]

Thank you for posting this @jypeter! This update sounds useful for those who currently use conda (and not mamba) and would like the significant speed advantage of mamba without needing to install it.

[pochedls]

Testing Feedback

Overall, I hit no major issues or unexpected errors during testing. Most of my suggestions are related to documentation. I stumbled onto out-of-date documentation (without specifically trying to review documentation) – so I think we should all try to do a systematic review of the docs (I'll help).

Temporal Bounds Updates

• This is not a problem, but I wanted to note that the API for add_bounds and add_missing_bounds is slightly (but unexpectedly different): the arguments are T versus axes=["T"], respectively. I think this is still okay to leave as-is.
• I recommend changing the upper bound to 21 for the day frequency (here) and 300 for the month frequency (here). There were instances where the incorrect frequency was inferred because the data had a 360_day calendar.
• It is pretty impossible to know exactly how to handle sub-daily bounds. We have them start at 00:00 and end at 00:00 (following CDAT). We should include this in the documentation here by adding:

If time steps are sub-daily, then the bounds will begin at 00:00 and end at 00:00 of the following day. For example, for 3-hourly data, the bounds would be ["01/01/2000 00:00", "01/01/2000 03:00"], ["01/01/2000 03:00", "01/01/2000 06:00"], ..., ["01/01/2000 21:00", "02/01/2000 00:00"].

Axis / Grid / Regridding updates

• attrs is repeated in create_axis here
  • addressed with [Doc]: Remove extra attrs docstring for create_axis #518 and Fixes duplicate parameter entry #532)
• The vertical regrid method had out-of-date documentation for Create destination grid. And here too.
• In horizontal_xesmf, should we be referencing xcdat.regridder.XESMFRegridder? Can all of that documentation be placed directly in horizontal_xesmf?

[tomvothecoder]

Thanks Steve, I'm glad you were able to test and noted all of these helpful details.

I am behind on reaching out to people for their availability to help test xCDAT, but will do it some time this week.

[tomvothecoder]

This is not a problem, but I wanted to note that the API for add_bounds and add_missing_bounds is slightly (but unexpectedly different): the arguments are T versus axes=["T"], respectively. I think this is still okay to leave as-is.

add_bounds() and get_bounds() are both based on cdms2, which I think handles one axis at a time. add_missing_bounds() can handle one axis or multiple axis.

I recommend changing the upper bound to 21 for the day frequency (here) and 300 for the month frequency (here). There were instances where the incorrect frequency was inferred because the data had a 360_day calendar.

I'm trying to understand why 21 and 300 are specifically needed to handle "360_day" calendars. Can you elaborate here?

It is pretty impossible to know exactly how to handle sub-daily bounds. We have them start at 00:00 and end at 00:00 (following CDAT). We should include this in the documentation here by adding:

If time steps are sub-daily, then the bounds will begin at 00:00 and end at 00:00 of the following day. For example, for 3-hourly data, the bounds would be ["01/01/2000 00:00", "01/01/2000 03:00"], ["01/01/2000 03:00", "01/01/2000 06:00"], ..., ["01/01/2000 21:00", "02/01/2000 00:00"].

I'll add this documentation, thanks.

---

Axis / Grid / Regridding updates

* `attrs` is repeated in `create_axis` [here](https://github.com/xCDAT/xcdat/blob/main/xcdat/regridder/grid.py#L641-L642)
  
  * addressed with [[Doc]: Remove extra `attrs` docstring for `create_axis` #518](https://github.com/xCDAT/xcdat/issues/518) and [Fixes duplicate parameter entry #532](https://github.com/xCDAT/xcdat/pull/532))

* The `vertical` regrid method had out-of-date documentation for [Create destination grid](https://github.com/xCDAT/xcdat/blob/main/xcdat/regridder/accessor.py#L373-L375). And [here too](https://github.com/xCDAT/xcdat/blob/main/xcdat/regridder/xgcm.py#L164).

* In [`horizontal_xesmf`](https://github.com/xCDAT/xcdat/blob/main/xcdat/regridder/accessor.py#L125), should we be referencing `xcdat.regridder.XESMFRegridder`? Can all of that documentation be placed directly in `horizontal_xesmf`?

These should all be addressed by #535.

[pochedls]

@tomvothecoder - this section of code is differentiating timesteps of hours, days, and months. The current code says:

Minimum Time Step Criteria	Classification
t < 24 hours	hour
t ≥ 1 day & t < 28 days	day
t ≥ 28 day & t < 365 days	month
Not any of the above	year

A 360 day calendar for yearly time steps would have time deltas of 360 days (which would be incorrectly classified as one month rather than one year). The upper bound of the cutoffs are really close to the next frequency category. For example, if a time step is 27 days, we call it a day time step, even though it's probably closer to month. Changing these categorizations makes us a little less susceptible to misclassification due to weird data. That's why I suggested changing the upper bounds for the day and month classifications.

[tomvothecoder]

@pochedls Thanks for clarifying. I thought 21 for day and 300 for month were based on how coordinates were setup for "360_day" calendars or something to that extent. Instead, they are general numbers that reduce the likelihood of misclassification.

[tomvothecoder]

Test bounds capabilities (@lee1043 and @tomvothecoder)

1. Dataset.bounds.add_missing_bounds()
   • Test case: Validate "X", "Y", and "Z" axes bounds uses coordinates as midpoints
   • Test case: Validate "T" axis bounds uses frequency of time coordinates
2. Dataset.bounds.add_time_bounds()
   • Test case: Validatemethod="freq" adds time bounds using frequency of time coordinates
     • Test with hourly daily, monthly, and yearly data
   • Test case: Validatemethod="midpoint" adds time bounds using time coordinates as midpoints
     • Test with hourly daily, monthly, and yearly data

[tomvothecoder]

This should be good to go. The unit tests already do a good job of covering these. I improved documentation in this PR #540 and performed other updates based on @pochedls feedback above.

[tomvothecoder]

Test grid creation (@mzelinka, @durack1, @chengzhuzhang, @tomvothecoder)

1. xcdat.create_grid
   • Test case: Create a grid dataset using xcdat.create_axis to create x, y and z arguments

Test vertical regridding (@mzelinka, @durack1, @chengzhuzhang)

1. Dataset.regridder.regrid
   • Test case: xgcm linear method (default) with various options
   • Test case: xgcm log method with various options
   • Test case: xgcm conservative method with various options

[tomvothecoder]

In my e3sm_diags refactoring work, I implemented _hybrid_to_plevs() and _pressure_to_plevs() which both call the vertical regridder and create_grid APIs. I caught a few bugs (#519, #539) that are fixed on main.

Other than that, everything else was smooth. I found create_grid easy to use by following the API documentation and the Jupyter Notebook example for vertical regridding. Results were close on the datasets I tested between the new and old versions of the functions in e3sm_diags (max 1e-6 abs and 1e-8 rel diffs). I only tested the xgcm log method though.

[chengzhuzhang]

create_grid works well for me. When following through the API documentation I found a typo, here (datatime --> datetime)

[tomvothecoder]

Thanks @chengzhuzhang. I'll open up an issue for that (#549)

[chengzhuzhang]

Thank you @tomvothecoder for open up an issue.

For vertical regridding, I tested the vertical regridding notebook with various options, covering log, linear, conservative methods. Again, the testing went without error, and I only spotted minor issues with docs. I will attempt a PR to suggest changes, and also fix #549

[chengzhuzhang]

Another minor note, when running regridding-vertical.ipynb. I got warning:

[WARNING]: bounds.py(add_missing_bounds:186) >> The nlat coord variable has a 'units' attribute that is not in degrees.

This seems to be that get_dim_coords is interpreting nlat as coord variable in addition to lat and add_missing_bounds is trying to add bounds for nlat?

[tomvothecoder]

Test Spatial Averaging (@crterai, @AaronDonahue, @pochedls)

1. Dataset.spatial.average()
   • Test case: Works with datasets with a prime meridian bounds cell that is not monotonically increasing or decreasing (e.g., [359, 1] or [1, 359] instead of [-1,1] or [1, -1]
     • Refer to this comment on CF convention by @taylor13)
     • For this case, the function _align_lon_to_360 should be called as a workaround

[crterai]

I tried the following with xcdat=0.6.0rc1 and it gave answers that looked reasonable:

import xcdat
import numpy as np
# Load SCREAMv1 regridded files with the Surface related output
directory_prefix='/global/cfs/cdirs/e3sm/terai/SCREAM/v1_production'
f_Jan_SurfVars=xcdat.open_mfdataset(directory_prefix+'/ne1024pg2_ne1024pg2_DY2/output.scream.SurfVars.INSTANT.nmins_x15.2020-01*nc')

varname = "T_2m"
filespec = f_Jan_SurfVars
a = np.array(filespec.spatial.average(varname, lat_bounds=(0,10),axis=["X","Y"])[varname].sel(time=slice('2020-01-20 00:00:00','2020-01-28 23:59:59')))
b = np.array(filespec.spatial.average(varname, lat_bounds=(80,90),axis=["X","Y"])[varname].sel(time=slice('2020-01-20 00:00:00','2020-01-28 23:59:59')))
errmax = np.amax(np.abs(a-b))
print("%e, %e, %e" %(a[0],b[0], errmax))

2.986010e+02, 2.476828e+02, 5.852430e+01

[tomvothecoder]

I think we're good to go with this specific test case.