Skip to content

Generalize plotting script for sigma tau.#11

Closed
jwbishop wants to merge 7 commits intouafgeotools:masterfrom
jwbishop:master
Closed

Generalize plotting script for sigma tau.#11
jwbishop wants to merge 7 commits intouafgeotools:masterfrom
jwbishop:master

Conversation

@jwbishop
Copy link
Contributor

@jwbishop jwbishop commented Nov 25, 2019

Note changes to the arguments!

Flag for sigma_tau added. Additionally, optional flag for plotting the M(d)CCM on it's own subplot also added. Tested and consistent with wlsqva_proc.

Note: This update isn't 100% complete. I'm note sure yet what the (currently silent) argument of mcthresh does.

@jwbishop
Copy link
Contributor Author

Concerning mcthresh, it looks like the indices of array_thresh would be helpful?

Copy link
Member

@liamtoney liamtoney left a comment

Choose a reason for hiding this comment

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

@jwbishop I made a few mostly minor suggestions. We probably should go thru and change every instance of az to baz in the array plotting script...

vplot = 1
bplot = 2
splot = bplot
if ccmplot is not False:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ccmplot is not False:
if ccmplot:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.


# Start Plotting
# Plot Trace
fig1, axarr1 = plt.subplots(num_subplots, 1, sharex='col')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fig1, axarr1 = plt.subplots(num_subplots, 1, sharex='col')
fig1, axs1 = plt.subplots(num_subplots, 1, sharex='col')

# Plot Trace
fig1, axarr1 = plt.subplots(num_subplots, 1, sharex='col')
fig1.set_size_inches(10, 9)
axs1 = axarr1.ravel()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
axs1 = axarr1.ravel()

Copy link
Member

Choose a reason for hiding this comment

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

Unneeded

# Colormap
cm = 'RdYlBu_r'
# Colorbar/y-axis for MdCCM
cax = 0.2, 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cax = 0.2, 1
cax = (0.2, 1)

Copy link
Member

Choose a reason for hiding this comment

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

This just makes things a little more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

axs1[2].set_ylim(.25,.45)
axs1[2].set_xlim(t[0],t[-1])
# Plot MdCCM on its own plot
if ccmplot is not False:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ccmplot is not False:
if ccmplot:

baz: Array of back-azimuth estimates
ccmplot: Flag to plot the Mean/Median cross-correlation
maxima values on the y-axis in addition to the color scale.
sigma_tau: Array of sigma_tau values
Copy link
Member

Choose a reason for hiding this comment

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

Is sigma_tau just a boolean? This docstring entry is confusing. If boolean, it should be default False. If it can be an array, it should be default None.

Copy link
Member

Choose a reason for hiding this comment

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

It should be an array

t: Array processing time vector
mdccm: Median cross-correlation maxima
vel: Array of trace velocity estimates
baz: Array of back-azimuth estimates
Copy link
Member

Choose a reason for hiding this comment

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

We have baz here, but az in the function definition and elsewhere...

axs1[bplot].set_ylabel('Back-azimuth\n [deg]')

# Plot sigma_tau
if sigma_tau is not False:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if sigma_tau is not False:
if sigma_tau:

@@ -1,4 +1,4 @@
#%% User-defined parameters
# %% User-defined parameters
Copy link
Member

Choose a reason for hiding this comment

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

Does this break the iPython cell?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in update.

@liamtoney
Copy link
Member

Note that this addresses #8

@davidfee5
Copy link
Member

This looks good to me and I can review it a bit more. Question though: should we try to generalize to accept LTS-flagged elements as well? Perhaps that can wait until the LTS code revisions are approved for public release.

example: array_plot(stf,tvec,t,mdccm,vel,az,mcthresh):
Generalized plotting script for velocity - back-azimuth array processing.

@ Author: David Fee
Copy link
Member

Choose a reason for hiding this comment

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

Remove all author names in functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering about this, ok!

Copy link
Member

@davidfee5 davidfee5 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Thanks!

fig1, axarr1 = plt.subplots(num_subplots, 1, sharex='col')
fig1.set_size_inches(10, 9)
axs1 = axarr1.ravel()
axs1[0].plot(tvec, st[0].data, 'k')
Copy link
Member

Choose a reason for hiding this comment

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

We could also add an option to plot the beam for a specified vel and baz, but perhaps that is best left for a future change

if ccmplot is not False:
sc = axs1[1].scatter(t, mdccm, c=mdccm,
edgecolors='k', lw=0.3, cmap=cm)
# axs1[1].plot([t[0], t[-1]], [mcthresh[0], mcthresh[1]], 'r--')
Copy link
Member

Choose a reason for hiding this comment

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

Delete?

mdccm: Median cross-correlation maxima
vel: Array of trace velocity estimates
baz: Array of back-azimuth estimates
ccmplot: Flag to plot the Mean/Median cross-correlation
Copy link
Member

Choose a reason for hiding this comment

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

Should it only be median xcorr? The rest of the code assumes it is mdccm. Is there a time we would want mccm? Probably not. Future option to think of: semblance instead of mdccm.

Copy link
Contributor Author

@jwbishop jwbishop Nov 25, 2019

Choose a reason for hiding this comment

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

Should it only be median xcorr? The rest of the code assumes it is mdccm. Is there a time we would want mccm? Probably not. Future option to think of: semblance instead of mdccm.

I'm actually a bit fan of the fisher statistics as well. We could modify to something like detection_metric. The code is agnostic to the actual meaning of that array. It's currently just used for the colorbar.

@jwbishop
Copy link
Contributor Author

This looks good to me and I can review it a bit more. Question though: should we try to generalize to accept LTS-flagged elements as well? Perhaps that can wait until the LTS code revisions are approved for public release.

I thought about adding that as well, but it I think it's best to initially make the simple generalization. Adding LTS plotting functionality adds two more import statements and a minimum 20 code lines.

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