search

NCAR / ADF

A unified collection of python scripts used to generate standard plots from CAM outputs.
Creative Commons Attribution 4.0 International
34 stars 29 forks source link

AdfData class for streamlined I/O in plotting scripts #269

Closed brianpm closed 1 month ago

brianpm commented 8 months ago

This PR introduces a new class called ADFData in lib/adf_dataset.py. This class is instantiated with an ADFDiag instance. It is intended to hold most of the boilerplate code that is in the plotting scripts to set paths and load data. It also abstracts the treatment of "obs" and "baseline" into something that looks more unified (at least to the plotting scripts). The upshot is that using this class can reduce the amount of code in plotting scripts. As examples, I've added zonal_mean_B.py and global_latlon_map_B.py. These are refactors of the current versions that now use ADFData, and both are quite a bit shorter in the B versions. In limited testing, I think they are producing the same output.

NOTE I see that create_climo_files.py also appears to be changed here. I don't know why. That should not be part of this PR... I did a sync with ADF so I thought I was completely up to date. I think the changes in that file were developed to help @justin-richling with an issue with the climatology years. The code looks right, but I wasn't intending it to be part of this PR. (It can be if we double check it.)

brianpm commented 2 months ago

@nusbaume -- Okay. I think I've got an updated version that is working.

I didn't rename the scripts because I'd like @justin-richling to confirm that the plots are actually working correctly.

In my limited testing using recent development runs, I had to also update adf_info.py because of the history file changes. So now it checks for time_bnds OR time_bounds and uses the hist_str to look for time series files. Feel free to exclude these changes is they are being handled by a different PR.

I also ran into the issue where doing the derived time series was hanging, so I added .compute() in adf_diag.py. If there's a more robust fix for that, this change can also be ignored.

justin-richling commented 2 months ago

@brianpm Looks like the plots for Lat/Lon and Zonal are the same between the old way and this way. Thanks for the clean up of the code!

brianpm commented 2 months ago

Okay, I think I've got everything in. It says there's conflicts with adf_info.py, but I think we want this version (@justin-richling took a look on my previous commit and noted that we would want a couple of the changes that I had accidentally removed that affect hist_str and time_bounds).