Skip to content

Addition of improved MODIS ice surface temperature observation converter program #858

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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

mollymwieringa
Copy link
Collaborator

Description:

The converter program written by Yongfei Zhang for transcribing MODIS ice surface temperature observations has been cleaned and standardized; the module has also been moved from the cice subdirectory of DART's obs_converter programs to the MODIS subdirectory. Changes to quickbuild.sh and the input.nml associated with MODIS have been implemented to facilitate the new version of the converter, MOD29E1D_to_obs.

An example preprocessing script to convert the MODIS IST hdf files to netcdf files with standardized lat/lon information and variables names is introduced in the shell_scripts directory. Note that this program requires rio_xarray, xarray, numpy, and geopandas packages to ascertain the lat/lon information associated with the 4km EASE grid on which the observations are provided. There are other potential solutions to this preprocessing requirement that may be more general; they have not been explored for this pull request.

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

Documentation changes needed?

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

Tests

MOD29E1D_to_obs builds successfully from quickbuild.sh. The python preprocessing script MOD29E1D_hdf_to_netcdf.py converts the example daily observation file (from Earthdata's HTTPS file system for MODIS IST observations) data/MOD29E1D_2000_02_24.EXAMPLE.hdf to a netcdf file. When that file is fed into the namelist for MOD29E1D_to_obs, the program writes an obs_seq.mod29e1d output file with expected observation, error, and qc values (O 100,000 observations total).

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

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

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an improved converter for MODIS ice surface temperature observations by cleaning up and standardizing the original converter program. Key changes include:

  • Addition of a new Python script to convert MODIS HDF observation files to netCDF.
  • Integration of geopandas-based CRS transformation to extract standardized lat/lon arrays.
  • Adjusted output variable transformations and file renaming to support the new converter version.
Files not reviewed (4)
  • observations/obs_converters/MODIS/MOD29E1D_to_obs.f90: Language not supported
  • observations/obs_converters/MODIS/MOD29E1D_to_obs.nml: Language not supported
  • observations/obs_converters/MODIS/work/input.nml: Language not supported
  • observations/obs_converters/MODIS/work/quickbuild.sh: Language not supported
Comments suppressed due to low confidence (1)

observations/obs_converters/MODIS/shell_scripts/MOD29E1D_hdf_to_netcdf.py:44

  • [nitpick] The output file is currently hardcoded to 'test.nc', which seems temporary. Please revert to using the intended production filename format.
post_file.to_netcdf('test.nc')

@hkershaw-brown
Copy link
Member

There is some super out of date, documentation for things that don't work, and slightly comical documentation in the MODIS directory on main:

https://docs.dart.ucar.edu/en/latest/observations/obs_converters/MODIS/readme.html
is not linked to from anywhere, comes up if you search MODIS in the docs. It lists someones hopes and dreams for the future.

https://docs.dart.ucar.edu/en/latest/observations/obs_converters/MODIS/MOD15A2_to_obs.html

Has the title PROGRAM MOD15A2_to_obs twice.

Screenshot 2025-04-29 at 2 32 55 PM

okie dokie

Screenshot 2025-04-29 at 2 32 43 PM

true/not true? Why have a bunch of docs for this unsupported tool.

I'll go ahead an pull out https://docs.dart.ucar.edu/en/latest/observations/obs_converters/MODIS/readme.html in this request, and put MOD15A2_to_obs weirdness into a different issue.

This is not referenced by any docs, seems to be out-of-date (does
not list all modis docs)
@hkershaw-brown
Copy link
Member

@mollymwieringa
Hi Molly, question on the "from the cice subdirectory of DART's obs_converter programs to the MODIS subdirectory"

Is observations/obs_converters/MODIS/MOD29E1D_to_obs.f90 replacing
observations/obs_converters/cice/modis_ist_to_obs_netcdf.f90?

At the moment modis_ist_to_obs_netcdf.f90 is in this branch, but I believe this should be removed and your MODIS/MOD29E1D_to_obs.f90 is what people should be using?

Cheers,
Helen

Taken from comments in code.
Added links from list of converters (not sure why we have two of these)
@hkershaw-brown
Copy link
Member

Added documentation page in a2c140c

Viewable for the pull request:
https://dart-documentation--858.org.readthedocs.build/en/858/observations/obs_converters/MODIS/MOD29E1D_to_obs.html#index-0
Feel free to edit as needed, the pull request documention will get rebuild for each commit.

Copy link
Member

Choose a reason for hiding this comment

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

This is a pretty large (47MB) file to commit, and it is output from the converter, so not needed. I'd like to take this out of the git history. This will mean rewriting the history on this branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should I delete this file in the pull request (as a new commit)? Or would you need me to leave it in order to take it out of the git history?

Comment on lines +1 to +4
import rioxarray as rxr
import xarray as xr
import geopandas as gpd
import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

Can you send along what versions of these packages you are using.

With the following:

pip freeze
affine==2.4.0
attrs==25.3.0
certifi==2025.4.26
click==8.1.8
click-plugins==1.1.1
cligj==0.7.2
GDAL==3.10.3
geopandas==1.0.1
nump==5.5.5.5
numpy==2.2.5
packaging==25.0
pandas==2.2.3
pyogrio==0.10.0
pyparsing==3.2.3
pyproj==3.7.1
python-dateutil==2.9.0.post0
pytz==2025.2
rasterio==1.4.3
rioxarray==0.19.0
shapely==2.1.0
six==1.17.0
tzdata==2025.2
xarray==2025.4.0

I get

File "rasterio/_base.pyx", line 312, in rasterio._base.DatasetBase.init
rasterio.errors.RasterioIOError: '../data/MOD29E1D_2000_02_24.EXAMPLE.hdf' not recognized as being in a supported file format.

gdalinfo can print file info ok:

gdalinfo ../data/MOD29E1D_2000_02_24.EXAMPLE.hdf
Driver: HDF4/Hierarchical Data Format Release 4
Files: ../data/MOD29E1D_2000_02_24.EXAMPLE.hdf
Size is 512, 512
Metadata:
  ALGORITHMPACKAGEACCEPTANCEDATE=12-2004
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are the versions from the environment I used to write that program:
rioxarray== 0.18.2
xarray==2025.3.1
geopandas==1.0.1
numpy== 1.26.4

Copy link
Member

Choose a reason for hiding this comment

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

@hkershaw-brown note to self, need to use conda to get gdal hdf

(geo-env) hkershaw:shell_scripts > conda env export --from-history
name: geo-env
channels:

  • defaults
    dependencies:
  • gdal
  • python=3.11
  • rasterio
  • rioxarray
  • geopandas
  • libgdal-hdf4
    prefix: /Users/hkershaw/miniconda3/envs/geo-env

python MOD29E1D_hdf_to_netcdf.py MOD29E1D.A2000055.061.2020037150613.hdf 2020 07 08

@hkershaw-brown hkershaw-brown added the cice Sea Ice models label May 2, 2025
@mollymwieringa
Copy link
Collaborator Author

Hi Helen,

Yes, observations/obs_converters/MODIS/MOD29E1D_to_obs.f90 replaces observations/obs_converters/cice/modis_ist_to_obs_netcdf.f90?. That's my bad, I thought I had removed modis_ist_to_obs_netcdf.f90 before submitting the pull request.

Best,
Molly

@mollymwieringa Hi Molly, question on the "from the cice subdirectory of DART's obs_converter programs to the MODIS subdirectory"

Is observations/obs_converters/MODIS/MOD29E1D_to_obs.f90 replacing observations/obs_converters/cice/modis_ist_to_obs_netcdf.f90?

At the moment modis_ist_to_obs_netcdf.f90 is in this branch, but I believe this should be removed and your MODIS/MOD29E1D_to_obs.f90 is what people should be using?

Cheers, Helen

@hkershaw-brown
Copy link
Member

Hi Helen,

Yes, observations/obs_converters/MODIS/MOD29E1D_to_obs.f90 replaces observations/obs_converters/cice/modis_ist_to_obs_netcdf.f90?. That's my bad, I thought I had removed modis_ist_to_obs_netcdf.f90 before submitting the pull request.

Best, Molly

@mollymwieringa Hi Molly, question on the "from the cice subdirectory of DART's obs_converter programs to the MODIS subdirectory"
Is observations/obs_converters/MODIS/MOD29E1D_to_obs.f90 replacing observations/obs_converters/cice/modis_ist_to_obs_netcdf.f90?
At the moment modis_ist_to_obs_netcdf.f90 is in this branch, but I believe this should be removed and your MODIS/MOD29E1D_to_obs.f90 is what people should be using?
Cheers, Helen

awesome thanks Molly, I've give those a spin.

@mollymwieringa
Copy link
Collaborator Author

@hkershaw-brown
Hi Helen!

I went ahead and removed the old modis_ist_to_obs_netcdf files in the cice subdirectory. I left the obs_seq.mod29e1d file in case you needed or wanted me to do something specific with it (I don't totally understand "rewriting the git history", but dw i'm off to read about it).

There are some additional commits for cleaning up some extraneous files that made it in during my committing-- I don't think you need to worry about them (they are not in the branch anymore).

Let me know if I missed anything!

Comment on lines 28 to 29
pre_file['lat'] = ({'x','y'}, lat)
pre_file['lon'] = ({'x','y'}, lon)
Copy link
Member

Choose a reason for hiding this comment

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

I think x,y is backwards here or in MOD29E1D_to_obs.f90 looking at the output netcdf file:

netcdf test {
dimensions:
        y = 1906 ;
        x = 1328 ;
variables:
        double tsfc(y, x) ;
                tsfc:_FillValue = NaN ;
        double y(y) ;
                y:_FillValue = NaN ;
        double lat(x, y) ;
                lat:_FillValue = NaN ;
        double lon(x, y) ;
                lon:_FillValue = NaN ;
        double x(x) ;
                x:_FillValue = NaN ;

! allocate arrays for lat/lon and sea ice temperature
allocate( lat(axdim, aydim))
allocate( lon(axdim, aydim))
allocate(seaice_temperature(axdim, aydim))

@hkershaw-brown
Copy link
Member

@hkershaw-brown Hi Helen!

I went ahead and removed the old modis_ist_to_obs_netcdf files in the cice subdirectory. I left the obs_seq.mod29e1d file in case you needed or wanted me to do something specific with it (I don't totally understand "rewriting the git history", but dw i'm off to read about it).

There are some additional commits for cleaning up some extraneous files that made it in during my committing-- I don't think you need to worry about them (they are not in the branch anymore).

Let me know if I missed anything!

Hi Molly,

Are you working on this branch today? If not I think I can rebase to rm the obs_seq.mod29e1d file from the commit it came in (a71f303) and leave the rest of the history alone. I'll need to force push so just checking that you don't have work in progress on this branch before I do that.
I pushed this branch to have a back up to https://github.com/hkershaw-brown/DART/tree/ice_converters in case I break anything.

Cheers,
Helen

@mollymwieringa
Copy link
Collaborator Author

I can stay off this branch today! I'll fix the (x, y) allocation issue once you're done.

@hkershaw-brown
Copy link
Member

I can stay off this branch today! I'll fix the (x, y) allocation issue once you're done.

I ended up just git rm obs_seq.mod29e1d. Rebasing or using git filter-repo was rewriting to much history to be worth worrying for this file (it is not crazy big).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cice Sea Ice models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants