search

rom-py / rompy

Relocatable Ocean Modelling in PYthon (rompy) combines templated cookie-cutter model configuration with various xarray extensions to assist in the setup and evaluation of coastal ocean model
https://rom-py.github.io/rompy/
BSD 3-Clause "New" or "Revised" License
2 stars 9 forks source link

Dimensions and coordinates maybe mixed up in SfluxSource #81

Open benjaminleighton opened 2 months ago

benjaminleighton commented 2 months ago

Code at

https://github.com/rom-py/rompy/blob/45fb70a713bf69f0c2e55862c6c3af4ead64fbf5/rompy/schism/data.py#L68-L86

renames dimensions to ny_grid and nx_grid using the datasetcoords object but I think this is a problem for multiple reasons:

  1. If this method runs twice it will fail because the dimensions have already been renamed
  2. It doesn't allow the dimensions to be alread called ny_grid and nx_grid
  3. lon, lat = np.meshgrid(ds[self.coords.x], ds[self.coords.y]) uses the coords.x and coords.y as coordinates correctly but the implication is that initially dimensions and coordinates are called the same thing. If they aren't called the same thing later cropping will fail because the indexer used is attempting to slice against the dimension and not the coordinate

Attached is a variation of the schism_procedural notebook that uses a barra like synthetic dataset that replicates the failure if the variables and coordinates aren't named the same thing problem I'm having using schism with barra data.

schism_procedural_1.zip

Initially I thought a solution to this was 

73a74
>         ds = ds.rename_dims({self.coords.y: "ny_grid", self.coords.x: "nx_grid"})
75,76c76,77
<         ds["lon"] = ((self.coords.y, self.coords.x), lon)
<         ds["lat"] = ((self.coords.y, self.coords.x), lat)
---
>         ds["lon"] = (("ny_grid", "nx_grid"), lon)
>         ds["lat"] = (("ny_grid", "nx_grid"), lat)

that is to remove the rename_dims and assign coords using coords for lon and lat but I don't think that works and gives odd dimensions in the resulting data.

benjaminleighton commented 2 months ago

I tried removing the logic alltogether from the SfluxSource ds function, things apparently work in that case but I don't think the dimensions and coordinates written to the schism sflux netcdf are correct. I think we need to implement coordinate and maybe dimension renaming in either get of SfluxSource or of SCHISMDataSflux?