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

Key metrics #118

Merged
merged 12 commits into from
Jul 31, 2024
Merged

Key metrics #118

merged 12 commits into from
Jul 31, 2024

Conversation

mnlevy1981
Copy link
Collaborator

@mnlevy1981 mnlevy1981 commented Jul 18, 2024

Put together a "key metrics" example that can be run quickly for a single CESM case to look at a small number of important diagnostics. The first pass at this adds a land-ice notebook, but more notebooks are coming.

All Submissions:

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you lint your code locally prior to submission?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

This is the first notebook that runs under the key_metric example
Removed the CESM3 vs CESM2 comparison section, refactored code to avoid
duplication.

Still to do: create utils.py so additional notebooks can share these refactored
functions. Probably some other code clean-up as well...
@TeaganKing
Copy link
Collaborator

@mnlevy1981 would it be helpful for me to review and then bring in the land-ice notebook as is, or should we wait until additional notebooks are ready to bring in?

@TeaganKing TeaganKing added high priority documentation Improvements or additions to documentation key metrics labels Jul 23, 2024
@mnlevy1981
Copy link
Collaborator Author

I have a few more changes to make for this one (pulling several functions out into a utils.py module and some other clean-up), then we should get feedback from @Katetc and @gunterl . I'm also tempted to move from netCDF4 calls to xarray to be more in line with other CUPiD notebooks.

One other thing to note: this notebook currently reads output from coupler history files, but I think Gunter said that work is in progress to get those fields into the CISM history files -- depending on the timeline for that, we should either pull in the notebook after it's cleaned up or wait until we can update the location of the history files to read.

Created a single utils module for now
1. Reorganized cells and added markdown headers (parameter configuration, set
up grid, make datasets, generate plots).
2. Fixed issue with plotting base maps (tested by comparing first 40 years of
run to last 40 years of run)
3. Added years included in climatology to the title of plots
Commented out the atmosphere, ocean, land, and sea ice section definitions and
added a section for land ice.
@mnlevy1981
Copy link
Collaborator Author

This is ready for review, though I think utils.py probably needs a bit of work. Do we want to move plotting routines to a new module and keep utils.py for computational routines? Do we want to reorganize utils.py so the routines called from the notebook are at the top? As far as output goes, the map looks like

SMB vs obs

and the time series looks like

SMB time series

Note that I added the years included in the climotology to the map title and the legend; to test the "compare against a base case" (which is turned off by default), I compared years 62-101 to years 43-82 of the same run and realized it was useful to have different labels for those different plots.

@mnlevy1981 mnlevy1981 marked this pull request as ready for review July 24, 2024 22:36
@mnlevy1981 mnlevy1981 assigned Katetc and TeaganKing and unassigned Katetc and TeaganKing Jul 24, 2024
@mnlevy1981
Copy link
Collaborator Author

(sorry for the assigning / unassigning, I thought I was requesting reviews)

@TeaganKing
Copy link
Collaborator

Thanks for all your work on this @mnlevy1981 ! I'm aiming to look at it tomorrow!

@TeaganKing
Copy link
Collaborator

A few comments:

  • Should these utils all necessarily be in the glc directory or is another more general location appropriate?
  • As we discussed today, converting netcdf4 --> xarray calls could also be useful.
  • I can do some more thorough testing later but figured it may be worthwhile to make these larger comments first

@mnlevy1981
Copy link
Collaborator Author

  • Should these utils all necessarily be in the glc directory or is another more general location appropriate?

I think we should keep them in glc until we have an example from another component that needs the same functions (or a generalized form that can be used both for land ice and the second component with different arguments). I'm not sure where the common utilities would go, though... my first thought was cupid/ but that directory is for cupid-dev rather than cupid-analysis.

  • As we discussed today, converting netcdf4 --> xarray calls could also be useful.

I'll work on this, and will probably end up with some of the functions we talked about (like a cupid_open_dataset that wraps xr.open_dataset or xr.open_mfdatasets). At which point the first bullet is worth revisiting, because that certainly wouldn't belong in the glc/ directory :)

  • I can do some more thorough testing later but figured it may be worthwhile to make these larger comments first

Thanks! Verifying you can run the notebook on my branch would be great, but given the changes coming to address your first two points I don't think you need to dive too deep at this point

Started replacing netcdf4 calls with xarray equivalents. I think the
observational dataset read is still entirely netcdf4-based, so that needs to be
updated. Also, I want to try calling xr.open_mfdataset() once per case and then
passing datasets rather than doing the read twice (once for temporal mean, once
for spatial mean).
Read in 40 years of SMB data with read_cesm_smb(), and then use xarray
functions (basically taking means or sums over different dimensions) to compute
the different climatologies. This greatly reduces the number of functions
needed in utils - it's just read_cesm_smb() and some plotting routines.
@mnlevy1981
Copy link
Collaborator Author

As of 5b6e3d5 I think this is really ready for review. I've gotten rid of the netcdf4 imports - we use xarray to open the datasets and then da.sum() and da.mean() instead of their numpy equivalents.

Copy link
Collaborator

@TeaganKing TeaganKing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to run this successfully and replicate the plots that you showed.

One other somewhat odd feature is that the output is located in "CUPiD/examples/key_metrics/computed_notebooks/key_metrics"-- is there a reason for this second 'key_metrics' directory?

examples/nblibrary/glc/utils.py Show resolved Hide resolved
@mnlevy1981
Copy link
Collaborator Author

One other somewhat odd feature is that the output is located in "CUPiD/examples/key_metrics/computed_notebooks/key_metrics"-- is there a reason for this second 'key_metrics' directory?

I thought we had an issue ticket about this, but I can't find it. I'd really like to clean up the names of the directories cupid-run creates -- maybe CUPiD_out/ with computed_notebooks/, computed_scripts/ (once we finish #93), and html/ (created by cupid-build rather than cupid-run).

Also renamed data -> da in plot_line() to be consistent with plot_contour()
Every panel was overwriting the location of the colorbar, so only the bias
contour levels were shown. Now the first two plots (with same vmin and vmax)
have a colorbar between them while bias plot colorbar is off to the right
@mnlevy1981 mnlevy1981 merged commit 53738a5 into NCAR:main Jul 31, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation high priority key metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants