Update codes for unstructured grid (CESM-SE)

[Duseong]

I updated the monet_accessor.py and combinetool.py

I ran the MELODIES-MONET for both CESM-FV and CESM-SE cases with these files, and it worked for both structured and unstructured grids, so I think it would be safe to be included, but let me know if there's any problem.

Most modifications are done in monet_accessor.py. I added several "if statements" to check whether the model output is structured ('lat' & 'lon') or unstructured ('ncol') grid. I also added "remap_nearest_unstructured" function, to find the closest model grid to the observation point. Currently, it's calculated using the center longitude and latitude values but maybe I will need to update it to consider grid corner values.

A slight modification was made for combinetool.py, I again added an if statement to call different functions for unstructured grid output. So, most of modifications were adding if statements to separate structured and unstructured grid cases.

[zmoon]

@Duseong I am wondering if instead of checking for an 'ncol' dimension every time it could be something more clear and generic, e.g.

• create a helper function, _has_unstructured_grid(da) or something
• check for an unstructured_grid attribute on the Dataset, which we could add on the monetio side when opening a CESM-SE file (or other unstructured grid files)
  • the helper function could even check for this as a first check, before looking for 'ncols' dim and/or lack of lat/lon dims

[zmoon]

Also could you run the pre-commit hooks on your branch? https://pre-commit.com/#quick-start It looks like linting would fail currently.

[Duseong]

Thanks for your comments! I removed 'ncol' check, and instead added "unstructured_grid" attribute on the Dataset. It is declared as "False" but for the unstructured grid, it is changed to True in driver.py like below:

    elif 'cesm_se' in self.model.lower(): 
        from new_monetio import read_cesm_se
        self.mod_kwargs.update({'var_list' : list_input_var})            
        self.mod_kwargs.update({'scrip_file' : self.scrip_file})
        print('**** Reading CESM model output...')
        self.obj, self.obj_scrip = read_cesm_se.open_mfdataset(self.files,**self.mod_kwargs)
        self.obj.monet.scrip = self.obj_scrip
        **self.obj.monet.unstructured_grid = True**

I ran pre-commit hooks and it finally showed:

Let me know if I need to do something more, thanks!

[zmoon]

It is declared as "False" but for the unstructured grid, it is changed to True in driver.py

To me it would make more sense to set it as True in the read_cesm_se reader, then it is already taken care of and (eventually) doesn't depend on M-M to work (i.e., would still work in a monet/monetio only workflow).

[zmoon]

Re: unstructured_grid attribute, I was actually thinking of adding it as a Dataset attribute in the reader function, like

ds.attrs["unstructured_grid"] = True

and then robustly check with

ds.attrs.get("unstructured_grid", False)

but what you did seems like it should work fine too. Do you have a preference? I guess with my method probably would want to use a more qualified name like 'mio_has_unstructured_grid' to separate it from dataset attrs.

[Duseong]

Thanks a lot for your comments! I actually wanted to do something similar to what you suggested, but because adding attribute to xarray dataset was failed (due to my limited knowledge), I tried to find a workaround, that was the previous commit that used the monet dataset attribute.

Since I like what you suggested, I changed the way of checking unstructured grid, adding a xarray attribute instead of adding the monet attribute.

[zmoon]

adding attribute to xarray dataset was failed

Yeah it's kind of cool, if you add something to the ds.attrs dictionary, it also magically gets added as an attribute to the Dataset instance, as long as it doesn't conflict with an existing attribute and can function as a valid Python name

[Duseong]

Yeah it's kind of cool, if you add something to the ds.attrs dictionary, it also magically gets added as an attribute to the Dataset instance, as long as it doesn't conflict with an existing attribute and can function as a valid Python name

Yes, what I tried was just using the dot, like ds.mio_has_unstructured_grid = True This approach was failed. It's really good to know the way of using ds.attrs dictionary, thanks again!

[Duseong]

I think we are safe to merge this, as you have tested it to work for you and it shouldn't affect other parts of monet

Yes I tested this code for both FV and SE grids, and it worked, so I think we are ready to merge this.

[Duseong]

Thanks for merging! It looks like I can't merge this.

[zmoon]

It looks like I can't merge this.

Yeah, it is limited to certain NOAA ARL people, but I am watching closely, so there shouldn't ever be a long wait.