ocean-eddy-cpt / gcm-filters

Diffusion-based Spatial Filtering of Gridded Data

https://gcm-filters.readthedocs.io/

Other

37 stars 24 forks source link

Add Binder badge to README #116

Closed rabernat closed 2 years ago

rabernat commented 2 years ago

Closes #112

codecov-commenter commented 2 years ago

Codecov Report

Merging #116 (3057f29) into master (7dfd3fe) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #116   +/-   ##
=======================================
  Coverage   98.41%   98.41%           
=======================================
  Files           9        9           
  Lines         944      944           
=======================================
  Hits          929      929           
  Misses         15       15

Flag	Coverage Δ
unittests	`98.41% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7dfd3fe...3057f29. Read the comment docs.

NoraLoose commented 2 years ago

Thanks for this PR, @rabernat! The Binder badge is awesome. I tested it, and it works.

Two questions:

3 out of our 6 example notebooks (docs/gpu.ipynb, docs/examples/example_tripole_grid.ipynb, docs/examples/example_vector_laplacian.ipynb) crash the Binder kernel because they quickly exceed the available 2 GB of memory. I'm hesitant to change the GCM documentation examples just to fit them into Binder. Opinions?
What conda(?) environment file does Binder use for our notebooks? All necessary packages seem to be there, but what if we require more in future example notebooks? It would be great if we could synchronize this somehow with the environment we are trying to set up in #120. I'm not sure how this would work because I'm not that familiar with Binder.

NoraLoose commented 2 years ago

@rabernat this PR is basically ready to be merged. Before doing so, I just had two questions, see my previous comment.

rabernat commented 2 years ago

Sorry for letting this hang.

I'm hesitant to change the GCM documentation examples just to fit them into Binder. Opinions?

It's not our problem that binder doesn't have enough memory. Perhaps we just add a note to the offending cells saying that they will probably crash if running on binder? Once Pangeo Binder comes back (hopefully within a month), we can maybe switch to that. It has more memory.

What conda(?) environment file does Binder use for our notebooks?

The environment is defined here: https://github.com/pangeo-data/pangeo-docker-images/tree/master/pangeo-notebook It is the standard environment used in Pangeo Cloud. This saves us the trouble of having to maintain our own environment (although we could always start doing that in the future if we want).

NoraLoose commented 2 years ago

Thanks Ryan! Switching to Pangeo Binder in the future would be great!

Perhaps we just add a note to the offending cells saying that they will probably crash if running on binder?

Hmm, I'm afraid these kind of notes would be distracting in the documentation. I will merge this as is (without added notes), unless anyone objects.