ESMG / gridtools

A collection of grid generation tools.
Other
17 stars 15 forks source link

Directly reference arrays in grid metrics #11

Closed angus-g closed 3 years ago

angus-g commented 3 years ago

The lat/lon arrays on a grid are actually DataArrays, and they cannot be used to define derived arrays in the computeGridMetricsSpherical routine. By referencing the underlying numpy arrays, the computation can proceed as expected.

It might also be worth removing a bunch of the commented-out code from this routine, and exercising computeGridMetricsSpherical in the test suite. I could do this if it's of interest too!

jr3cermak commented 3 years ago

Thank you! Did you make a spherical grid? It didn't work until this change? Do you have a small sample script I can use to reproduce the error?

I will pull this into the "dev" branch. We are leaning towards using xarray over numpy. Right now it is quite a hodgepodge between the two. Frankly, xarray brings along its own challenges between DataArray and Dataset.

There are a lot of duplicate functions. Instead of deleting no longer needed functions, please mark them as "DEPRECATED" or "QUEUE FOR DELETION" in comments/pydocs. We may need to keep different versions around to check against the ones we are using for reproducibility across platforms.

We can use all the help we can get!

Looks like we need more TODOs.

angus-g commented 3 years ago

I triggered the bug by going through the interactive application tutorial. The "Grid Info" tab showed empty because of the exception in that grid metric routine.

I agree that xarray is probably the better way to go, but there would be a pretty substantial refactor required, whereas this at least gets the grid generation working again (for me, anyway)!

Also, apologies for making this against the main branch! I'll rebase this PR onto dev and point it to that instead. It's my fault for not reading the CONTRIBUTING.md, although it's possible (I think) to also make an issue/PR template that would be more visible as it auto-populates in GitHub. It might also be a little easier to track the TODOs by using GitHub's Project functionality? But I don't want to come along and change the way you run the project! :)

jr3cermak commented 3 years ago

Please copy and paste the software_version attribute/string from the generated grid in the example.

I have been unable to reproduce the problem. The code works for me with or without this patch. So, I would like to see what versions of modules you have to try and track this down hopefully.

Example:

:software_version = "Cython 0.29.23; cartopy 0.19.0.post1; certifi 2021.5.30; cffi 1.14.5; cftime 1.5.0; chardet 4.0.0; click 7.1.2; cloudpickle 1.6.0; colorama 0.4.4; cycler 0.10.0; cython 0.29.23; cytoolz 0.11.0; dask 2021.6.2; distributed 2021.6.2; fiona 1.8.18; fsspec 2021.6.1; gridtools 0.3.0+2c15d5e; idna 2.10; importlib_metadata 4.5.0; kiwisolver 1.3.1; matplotlib 3.4.2; msgpack 1.0.2; netCDF4 1.5.6; numpy 1.21.0; packaging 20.9; pandas 1.2.5; platform linux-x86_64; psutil 5.8.0; pyparsing 2.4.7; pyproj 3.1.0; python 3.7.10; pytz 2021.1; requests 2.25.1; scipy 1.6.3; shapely 1.7.1; six 1.16.0; sortedcontainers 2.4.0; tblib 1.7.0; toolz 0.11.1; tornado 6.1; typing_extensions 3.10.0.0; urllib3 1.26.5; xarray 0.18.2; zipp 3.4.1" ;

angus-g commented 3 years ago

Weird! For me, I have:

Cython 0.29.24; IPython 7.24.1; async_generator 1.10; backcall 0.2.0; bleach 3.3.1; bokeh 2.3.3; bottleneck 1.3.2; cartopy 0.19.0.post1; certifi 2021.5.30; cffi 1.14.6; cftime 1.5.0; chardet 4.0.0; click 7.1.2; cloudpickle 1.6.0; colorama 0.4.4; colorcet 2.0.6; cycler 0.10.0; cython 0.29.24; cytoolz 0.11.0; dask 2021.7.0; datashader 0.13.0; datashape 0.5.4; decorator 5.0.9; defusedxml 0.7.1; distributed 2021.7.0; entrypoints 0.3; fiona 1.8.18; fsspec 2021.7.0; geoviews 0.0.0+g33876c88.gitarchive; gridtools 0.3.0+2c15d5e; holoviews 1.14.4; hvplot 0.7.3; idna 2.10; importlib_metadata 3.10.0; ipykernel 5.3.4; ipython_genutils 0.2.0; jedi 0.18.0; jinja2 3.0.1; jsonschema 3.2.0; jupyter_client 6.1.12; jupyter_core 4.7.1; jupyterlab_pygments 0.1.2; kiwisolver 1.3.1; llvmlite 0.36.0; markdown 3.3.4; markupsafe 2.0.1; matplotlib 3.3.4; matplotlib_inline 0.1.2; mistune 0.8.4; msgpack 1.0.2; multipledispatch 0.6.0; nbclient 0.5.3; nbconvert 6.1.0; nbformat 5.1.3; netCDF4 1.5.6; notebook 6.4.0; numba 0.53.1; numexpr 2.7.3; numpy 1.20.3; owslib 0.24.1; packaging 21.0; pandas 1.3.0; pandocfilters 1.4.3; panel 0.12.0; param 1.11.1; parso 0.8.2; pexpect 4.8.0; pickleshare 0.7.5; platform linux-x86_64; prompt_toolkit 3.0.17; psutil 5.8.0; ptyprocess 0.7.0; pycparser 2.20; pyct 0.4.8; pygments 2.9.0; pykdtree 1.3.4; pyparsing 2.4.7; pyproj 3.1.0; pyrsistent 0.17.3; python 3.7.10; pytz 2021.1; pyviz_comms 2.0.2; requests 2.25.1; scipy 1.6.3; shapely 1.7.1; six 1.16.0; sortedcontainers 2.4.0; tblib 1.7.0; testpath 0.5.0; toolz 0.11.1; tornado 6.1; tqdm 4.61.2; traitlets 5.0.5; typing_extensions 3.10.0.0; urllib3 1.26.6; wcwidth 0.2.5; webencodings 0.5.1; xarray 0.19.0; zipp 3.5.0

Notably from a quick scan, I have xarray 0.19.0 vs. 0.18.2. I can generate the grid per the tutorial without problem by downgrading. So I guess there's a bit of an API inconsistency here!

jr3cermak commented 3 years ago

Obtained a 2nd report of this issue from Olga Sergienko that looks very similar to this. I am going to proceed to upgrade to 0.19.0 and test all the examples. There may be other dereferences to fix. It is good to find the root cause.

Make a grid with the grid parameters. Generating regular lat-lon grid centered at (230.00, 0.00) on equator. Generated regular lat-lon grid between latitudes -15.00 15.00 Number of js=61 Traceback (most recent call last):

File "/Users/olga/gridtools/gridtools/examples/mkGridsExample01.py", line 77, in grd.makeGrid()

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/gridtools/gridutils.py", line 1023, in makeGrid self.computeGridMetricsSpherical()

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/gridtools/gridutils.py", line 548, in computeGridMetricsSpherical self.grid['dx'] = (('nyp', 'nx'), R * spherical.angle_through_center( (lat[ :,1:],lon[ :,1:]), (lat[: ,:-1],lon[: ,:-1]) ))

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/dataset.py", line 1563, in setitem self.update({key: value})

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/dataset.py", line 4208, in update merge_result = dataset_update_method(self, other)

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/merge.py", line 984, in dataset_update_method return merge_core(

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/merge.py", line 632, in merge_core collected = collect_variables_and_indexes(aligned)

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/merge.py", line 294, in collect_variables_and_indexes variable = as_variable(variable, name=name)

File "/opt/anaconda3/envs/gridTools/lib/python3.9/site-packages/xarray/core/variable.py", line 121, in as_variable raise TypeError(

TypeError: Using a DataArray object to construct a variable is ambiguous, please extract the data using the .data property.