NCAR / CUPiD

CUPiD is a “one stop shop” that enables and integrates timeseries file generation, data standardization, diagnostics, and metrics from all CESM components.
https://ncar.github.io/CUPiD/
Apache License 2.0
25 stars 24 forks source link

Compute land-atmosphere terrestrial coupling index in key_metrics #145

Closed megandevlan closed 1 week ago

megandevlan commented 1 month ago

All Submissions:

New Feature Submissions:

  1. [x] Does your submission pass tests? -- Not sure
  2. [x] Have you lint your code locally prior to submission? -- Not sure

Changes to Core Features:

mnlevy1981 commented 1 month ago

Thanks for opening this PR!

A few comments before diving into the nuts and bolts:

  1. Can you please run pre-commit (and set it up to run on all future commits from your CUPiD directory)? Instructions are in the wiki (specifically, in the Contributor's Guide). I would recommend running both pre-commit run --all-files and pre-commit install from (cupid-dev), and then committing additional changes to the branch from that environment as well
  2. There are a lot of empty cells in the notebook that can probably be removed. Also, adding the hide-input tag will hide the input cell when creating the JupyterBook webpage while hide-cell will hide both the input and output; you might want to add one of those or the other to each cell in your notebook to make the webpage a little easier to view
  3. Can you please remove all the output from the notebook in examples/nblibrary/? Just a simple "restart kernel and clear output" before saving will keep it as a blank template.
  4. I see a bunch of /glade/derecho/scratch/mdfowler/tmp/ipykernel_13813/1925081246.py:42: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.), and I think the warnings are pointing to lat_fluxnet[iStation] = siteInfoDF['LOCATION_LAT'].values[indStation] and lon_fluxnet[iStation] = siteInfoDF['LOCATION_LONG'].values[indStation] in [9]; can you figure out the correct indexing to make sure both sides of the equation are scalars and avoid this warning? (I'm happy to help you play with that if you'd like, though I won't have a chance until tomorrow)

Once those bigger updates are made, I'll look more closely at the notebook

megandevlan commented 1 month ago

I've made most of those changes. For some reason, the tags for hide-input and hide-output aren't working as expected, but otherwise I think it's better!

megandevlan commented 1 week ago

I'm not sure where the error messages are coming from, but I can confirm that the website looks good now! And things are updated to point to the right file name.

mnlevy1981 commented 1 week ago

This looks great! One more question before I merge it -- it looks like you added examples/nblibrary/lnd/image.png to the repository. Is that on purpose? I don't think the image is used anywhere.

megandevlan commented 1 week ago

Nope I didn’t add that on purpose, but I think it auto-generated when I ran the script. I’ll delete it and re-commit :)