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

Update plotting routines in QC script #355

Merged
merged 5 commits into from
Aug 24, 2019

Conversation

mattdturner
Copy link
Contributor

@mattdturner mattdturner commented Aug 22, 2019

PR checklist

  • Short (1 sentence) summary of your PR:
    Fix an issue where the 2-stage failure map had a flipped x-axis map. Also add new plotting capability.
  • Developer(s):
    @mattdturner
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Tested the QC script plotting capability on 4 datasets on 1 server.
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

@mattdturner
Copy link
Contributor Author

mattdturner commented Aug 22, 2019

The documentation has not been updated yet. I wanted some feedback on the capability that is being added before updating documentation. This PR adds a new --plot option that will plot the mean ice thickness in a spatial map, similar to the figures below. It creates plots for the baseline dataset, test dataset, as well as the difference.

image

image

image

This addition of plotting might potentially address a portion of #347. It also fixes the issue where the plot of 2-stage failures had the longitudes flipped (the map looked mirrored) as shown in #347 .

@apcraig
Copy link
Contributor

apcraig commented Aug 22, 2019

Is there a reason not to have the --plot option on all the time?

The plots look a bit odd with the whitespace between data, but I assume that's an artifact of the plotting package, correct?

These plots are in addition to a plot of the pass/fail status per gridcell?

@eclare108213
Copy link
Contributor

This is super helpful, thanks!

@mattdturner
Copy link
Contributor Author

Yeah, the whitespace is a result of the fact that I am using the scatter function to create the plots. I could set up the script to create filled contour plots:

image

or pcolor plots:

image

Or, I could also setup the script to allow all 3 and have the user select which one with an optional argument (while setting a default if no argument is passed). If this were solely a plotting script, I would likely do that. However, I'm not sure if we want plotting-specific command line options for a script whose main functionality is to do the QC test. I wonder if that might get confusing...

I originally set it up to have plotting as an option because I thought that plots would only be desired if tests failed. I could set it to always plot the ice thickness data, and just include it in a try block so that the script continues if the plotting fails (i.e., if the user doesn't have the necessary Python packages).

And yes, this is in addition to the plot of pass/fail grid cells for the 2-stage test

@apcraig
Copy link
Contributor

apcraig commented Aug 23, 2019

This all is great. I am fine with any of the plotting options as the default. I guess I like pcolor the most (but just one opinion) and I would probably turn on the --plot all the time with a package check. But I'm also happy to merge the PR now.

@mattdturner
Copy link
Contributor Author

I'll have to update the documentation prior to merging the code. I'll go ahead and do that today and let you know when its ready.

Matthew Turner added 2 commits August 23, 2019 19:30
… optional plot_type argument, which can plot the data using scatter, contour, or pcolor. The default map (if no plot_type argument is passed) is pcolor.
@mattdturner
Copy link
Contributor Author

I added a brief paragraph to the documentation that mentions the plotting capability. I also modified the script to always attempt to plot the data (I removed the --plot option), and added a new --plot_type argument that accepts "scatter", "contour", or "pcolor" (with "pcolor" set as the default).

@apcraig apcraig merged commit c0cad11 into CICE-Consortium:master Aug 24, 2019
@mattdturner mattdturner deleted the qc_map_flip_axis branch August 26, 2019 12:00
@dabail10
Copy link
Contributor

This script is dying on cheyenne with X window related errors. Is there a python version issue here or do I need to do something different with my X window environment?

Dave

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.

4 participants