Skip to content
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

Fixing data access for notebooks 1 and 5 #38

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

Conversation

rmshkv
Copy link
Member

@rmshkv rmshkv commented Jul 19, 2023

Fixes issue #37 - notebooks 1 and 5 are now functional!

(Notebooks 2-4 are still broken due to data access issues, need a new place to host or some other solution).

@brian-rose
Copy link
Member

@rmshkv this push should fix the link failure so we're now pointing toward the correct binder.

I also updated the notebook execution settings in the config file so that the build will fail when unflagged exceptions occur in the notebooks (this is now standard in our cookbook template).

Since notebooks 2,3,4 are still not ready to run, I removed them from the TOC file for now. This way, we don't try to execute them at each build and encounter a failure.

@brian-rose
Copy link
Member

Build failure on GitHub Actions in notebook 1 at the line

gateway = Gateway()

I will take a closer look.

@brian-rose
Copy link
Member

@rmshkv the code is failing because we don't have Dask Gateway configured in the execution environment on GitHub Actions. We plan to have this functionality available on the Pythia Binder, but we're not there yet either unfortunately.

I managed to run notebook 1 (albeit very slowly) locally on my laptop by using

from distributed import LocalCluster
cluster = LocalCluster()

Maybe the path forward is to include a switch like in the NA-CORDEX Data Visualization Cookbook that defaults to LocalCluster() but makes it easy to switch to Gateway() where available.

@brian-rose
Copy link
Member

Incidentally, I think there's a scientific error in the "Timeseries of Global Mean Sea Level" section. Shouldn't the global mean be weighted by area of grid cells?

@rmshkv
Copy link
Member Author

rmshkv commented Jul 25, 2023

Thanks for taking a look! I'm working on adding the different cluster options as in the NA-CORDEX notebook. Also good catch on the grid cell area, I'll take a look at that too.

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

Successfully merging this pull request may close these issues.

2 participants