xCDAT / xcdat

An extension of xarray for climate data analysis on structured grids.
https://xcdat.readthedocs.io/en/latest/
Apache License 2.0
101 stars 11 forks source link

Update regridding notebook for v0.7.0 #646

Closed chengzhuzhang closed 2 months ago

chengzhuzhang commented 2 months ago

Description

For addressing: https://github.com/xCDAT/xcdat/issues/589#issuecomment-2057988462 but focus only on horizontal and vertical regridding notebooks

Checklist

If applicable:

chengzhuzhang commented 2 months ago

Only a few minor changes that makes note books to run. Some changes noted:

  1. notebook setup: I had to include python -m ipykernel install --user --name xcdat_notebook --display-name xc070 to have the xcdat_notebook kernel to show up in Jupyter. Though I don't see this step in other example notebook.
  2. I added gsw-xarray package as a dependency which is needed by the vertical regridding notebook.
  3. In https://github.com/xCDAT/xcdat/blob/9193509919fe5123f989e2211d2517c266cbe097/docs/examples/regridding-horizontal.ipynb: Ln 8: created a new warning:/Users/zhang40/mambaforge/envs/xcdat_notebook/lib/python3.12/site-packages/xcdat/regridder/regrid2.py:195: RuntimeWarning: invalid value encountered in divide np.divide(, which I think is resulted by a recent change from @jasonb5, and the result figure seems to be deviated from the results on main (here) based my assessment with naked eyes. Not sure if this is expected.
pochedls commented 2 months ago

I don't mind including notebook setup instructions, but I had included the following text in order to avoid repeating the instructions across all notebooks:

Users can install their own instance of xcdat and follow these examples using their own environment (e.g., with vscode, Jupyter, Spyder, iPython) or enable xcdat with existing JupyterHub instances. The conda environment used in this notebook includes xcdat, xesmf, matplotlib, ipython, ipykernel, cartopy, and jupyter:

tomvothecoder commented 2 months ago

Only a few minor changes that makes note books to run. Some changes noted:

1. notebook setup: I had to include `python -m ipykernel install --user --name xcdat_notebook --display-name xc070` to have the xcdat_notebook kernel to show up in Jupyter. Though I don't see this step in other example notebook.

I was able to use the Python kernel from the xcdat_notebook environment in Jupyter. This kernel is listed as "Python 3 (ipykernel)", not xcdat_notebook. I don't think it is necessary to call the python -m ipykernel command for this case. The import xcdat line works fine in the screenshot below:

image

Commands used:

$ conda create -n xcdat_notebook -c conda-forge xcdat xesmf matplotlib ipython ipykernel cartopy nc-time-axis jupyter
$ conda activate xcdat_notebook
$ jupyter notebook
chengzhuzhang commented 2 months ago

@tomvothecoder I did not know Python3 (ipykernel) is the one with xcdat. I thought it is just bare-bone python3. I agree python -m ipykernel is not necessary, but when multiple kernels are with the same name, it can cause some confusion. And I'm still fine with registering the xcdat_note book with a specified name at this point.

tomvothecoder commented 2 months ago

@tomvothecoder I did not know Python3 (ipykernel) is the one with xcdat. I thought it is just bare-bone python3. I agree python -m ipykernel is not necessary, but when multiple kernels are with the same name, it can cause some confusion. And I'm still fine with registering the xcdat_note book with a specified name at this point.

If Jupyter is invoked from within a conda environment (xcdat_notebook in this case), the Python kernel of that environment is defaulted as "Python 3 (ipykernel)". I don't think there should be any other kernels listed unless the user manually adds them (e.g., python -m ipykernel), since the Jupyter instance is isolated within the conda env (not 100% sure though).

On the other hand, public Jupyter instances would probably have multiple kernels listed (e.g., NERSC Jupyter Hub) since kernels are manually registered to that public instance.

We can either 1) add a note on the notebooks to let the the user know Python3 (ipykernel) is the kernel of xcdat_notebook by default or 2) add the extra step to register the kernel in all of the notebooks, which is necessary if the tries to use the notebooks through a Jupyter instance invoked elsewhere and not xcdat_notebook.

tomvothecoder commented 2 months ago

I don't think there should be any other kernels listed unless the user manually adds them (e.g., python -m ipykernel), since the Jupyter instance is isolated within the conda env (not 100% sure though).

I just confirmed that no other kernels are registered in the Jupyter instance invoked by xcdat_notebook. The only one listed is "Python 3 (ipykernel)", which is the ipykernel of xcdat_notebook.

image

chengzhuzhang commented 2 months ago

This is what I see from my end...In my Jupyter instance, it includes even my old conda env registered with Jupyter.

Screen Shot 2024-04-18 at 10 35 34 AM
chengzhuzhang commented 2 months ago

We can either 1) add a note on the notebooks to let the the user know Python3 (ipykernel) is the kernel of xcdat_notebook by default or 2) add the extra step to register the kernel in all of the notebooks, which is necessary if the tries to use the notebooks through a Jupyter instance invoked elsewhere and not xcdat_notebook.

I think it is a good solution!

tomvothecoder commented 2 months ago

This is what I see from my end...In my Jupyter instance, it includes even my old conda env registered with Jupyter.

Thanks for the screenshot! Yeah I'm not sure why those kernels are registered with the Jupyter instance invoked through xcdat_notebook. Maybe that's how it's supposed to be.

In any case, I will update all the notebooks using both solutions above in a separate PR.

chengzhuzhang commented 2 months ago

This is what I see from my end...In my Jupyter instance, it includes even my old conda env registered with Jupyter.

Thanks for the screenshot! Yeah I'm not sure why those kernels are registered with the Jupyter instance invoked through xcdat_notebook. Maybe that's how it's supposed to be.

In any case, I will update all the notebooks using both solutions above in a separate PR.

Thank you for implementing it!

jasonb5 commented 2 months ago

@chengzhuzhang @tomvothecoder The warning /Users/zhang40/mambaforge/envs/xcdat_notebook/lib/python3.12/site-packages/xcdat/regridder/regrid2.py:195: RuntimeWarning: invalid value encountered in divide np.divide( is fine, it occurs when regridding with a mask and the sum of the input weights for a cell is zero, which is just missing data in that cell.

I do see the visual difference in the last example, this would be expected as I reworked how masking worked. The updated result should be more accurate, it appears to be visually.

chengzhuzhang commented 2 months ago

@chengzhuzhang @tomvothecoder The warning /Users/zhang40/mambaforge/envs/xcdat_notebook/lib/python3.12/site-packages/xcdat/regridder/regrid2.py:195: RuntimeWarning: invalid value encountered in divide np.divide( is fine, it occurs when regridding with a mask and the sum of the input weights for a cell is zero, which is just missing data in that cell.

I do see the visual difference in the last example, this would be expected as I reworked how masking worked. The updated result should be more accurate, it appears to be visually.

Got it. Thank you for the calcification @jasonb5

tomvothecoder commented 2 months ago

@chengzhuzhang @tomvothecoder The warning /Users/zhang40/mambaforge/envs/xcdat_notebook/lib/python3.12/site-packages/xcdat/regridder/regrid2.py:195: RuntimeWarning: invalid value encountered in divide np.divide( is fine, it occurs when regridding with a mask and the sum of the input weights for a cell is zero, which is just missing data in that cell.

I do see the visual difference in the last example, this would be expected as I reworked how masking worked. The updated result should be more accurate, it appears to be visually.

Great! @chengzhuzhang You can merge this PR if you think it is ready.