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

Global mean time series plotting script #334

Merged
merged 10 commits into from
Nov 19, 2024

Conversation

brianpm
Copy link
Collaborator

@brianpm brianpm commented Oct 15, 2024

Add a script to plot global mean time series (annual mean only right now).

Uses time series files directly. To do this, I added some functionality to adf_dataset.py to get time series data.

Adds a TimeSeries type of plot category to adf_variable_defaults.yaml.

Modifies plotting_functions.py to try to keep units in tact while doing spatial and annual averages.

Changes in adf_info.py is incidental (two print statements), and can be ignored.
The change in adf_web.py is literally whitespace and can be ignored.

# reference time series (DataArray)
ref_ts_da = adfobj.data.load_reference_timeseries_da(field)
# check if this is a "2-d" varaible:
has_lat_ref, has_lev_ref = pf.zm_validate_dims(ref_ts_da)
Copy link

@adagj adagj Nov 8, 2024

Choose a reason for hiding this comment

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

If the field doesn't exist (for some reason), the adfobj.data.load_reference_timeseries_da(field) will return a Nonetype, and
has_lat_ref, has_lev_ref = pf.zm_validate_dims(ref_ts_da)
fails, and ADF crashes.

I added

       ref_ts_da = adfobj.data.load_reference_timeseries_da(field)
        if ref_ts_da is None:
            print(
                f"\t Variable named {field} provides Nonetype. Skipping this variable"
            )
            continue
        # check if this is a "2-d" varaible:
        has_lat_ref, has_lev_ref = pf.zm_validate_dims(ref_ts_da)

which seems to work fine

Copy link
Collaborator

Choose a reason for hiding this comment

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

@adagj You're right, thanks for finding this! I can confirm this fix works

Additionally, if the ADF is comparing against obs, it will also trigger the Nonetype.

@brianpm We can just add this check and not have the time series plot made if comparing against obs, or I have coded up a quick fix to allow the time series plots to be created in the model vs obs case, but will just plot the test time series. I'm happy to share if this desired.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2024-11-18 at 3 13 33 PM (3)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justin-richling -- I think your solution to keep making the plot (just skip the reference) is the desired behavior. Can you incorporate your fix and merge it in?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@brianpm Will do!

@@ -684,7 +684,7 @@ def jinja_list(seas_list):

#List of ADF default plot types
avail_plot_types = res["default_ptypes"]

#Check if current plot type is in ADF default.
#If not, add it so the index.html file can include it
for ptype in plot_types.keys():
Copy link

Choose a reason for hiding this comment

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

There was a problem causing ADF to crash in the for loop starting on line 687 in adf_web.py

I needed to change

 for ptype in plot_types.keys():
                if ptype not in avail_plot_types:
                    avail_plot_types.append(plot_types)
                    

to

 for ptype in plot_types.keys():
                if ptype not in avail_plot_types:
                    avail_plot_types.append(ptype)
                    

Copy link
Collaborator

Choose a reason for hiding this comment

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

@adagj Thanks for catching this! Although this should've caught the TimeSeries plot type since it is included in lib/adf_variable_defaults.yaml. I'm curious why this crashed for you, would you be able to provide any further details when this crashed? Thanks!

Remove testing print statements
This will be important when variables that aren't in the ref case
Fix bug when variable doesn't exist in reference files and allow for model vs obs to continue, just plotting the test case.
Add check for variable unit from variable defaults yaml in case none are listed in time series file.

Also update check for `NoneType` for data arrays
@justin-richling
Copy link
Collaborator

justin-richling commented Nov 19, 2024

@brianpm This all looks good to me now. I've tested with obs and tested with with fake variables and it works now as intended.

@adagj Thanks again for the review and changes, we appreciate your time in helping!

@justin-richling justin-richling merged commit 380087b into NCAR:main Nov 19, 2024
7 checks passed
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.

3 participants