ENH: Updating plot in estimate_image_shift
#753
Conversation
|
|
| d = (np.array(phase_correlation.shape) - 1) // 2 | ||
| extent = [-d[1], d[1], -d[0], d[0]] | ||
| axarr[2].imshow(np.fft.fftshift(phase_correlation), | ||
| cmap=plt.cm.jet, extent=extent) |
There was a problem hiding this comment.
Why jet? I think anything would be better. You could use viridis, as it will be the new default anyway. Also strange that the colormap is set explicitly, but you can't pass your own value... Though I see the point - most likely the user will want to have the phase image plotted differently than the reference, and if just passing kwargs or something alike, then all will be the same. I still would vote for "anything-but-jet" :)
There was a problem hiding this comment.
viridis is only available in the very latest mpl release. I don't think that it is justified to restrict our compatibility to mpl > 1.5 just because of this.
+1000 on anything-but-jet, and +1 to using the default cmap that will end up being viridis for mpl >= 1.5
There was a problem hiding this comment.
Used jet initially to get it to feel like DM's drift correction, and then never thought to change it. Putting it as an optional kwarg is of course doable, but would expand the already very large function signature of Singal.estimate_shift2d(). Leaving it to default means it gets plotted in the same cmap as as the two other frames, and it would be nice to differentiate it. We could of course try to plot it as an outline plot? :) Something with reasonable contrast in the high-intensity regime would be at least be useful, as it can help differentiate local maxima that are close to the global max.
There was a problem hiding this comment.
How about putting this as a preference? It's not something where I would imagine people would change cmap from time to time, but they might have a general preference. However, I'm not sure what the guidelines are for making something a setting (e.g. if this is too trivial).
|
A drawback of "reuse" is that, when the operation time is short, the figure updates too fast for inspection. What about storing the images in |
If the operation is fast, it is less likely to need continuous feedback, but not completely unlikely; so yes, it would definitely be nice to plot it better/faster (does it flicker for you?). Doing it by blit is of course the best way.
That could be an option, but the reference/current is not needed as a new Signal since they're basically from the input signal (so it would be created only for the purpose of plotting). Further, an argument against this is the increased memory use. For large datasets, which is when the continuous monitoring would be the most useful, the increase in memory use would be significant. As such, it could be added as a future option, but I don't think it should replace the solution proposed. |
|
This PR doesn't do it claims to do:
Actually, it replaces the old behavior, which is fine for me as it was horrible :) However, the header must be amended to explain exactly what it does. I agree with @to266 in the "jet" issue. Making HyperSpy feel like DM it is not our aim. Also, I don't see why the correlation plot must use a different cmap. +1 on using the default cmap or to add a |
Strange, for me it works like I'd expect. Can you detail what you do, what you expect should happen, and what actually happens? There seem to be some effect of backend/interactivity, so if you could list those settings as well, it would help out a lot. |
|
|
vidartf
force-pushed
the
ENH_alignment_plots
branch
from
39f510a
to
0e2340f
Compare
Previously for Signal.estimate_shift2D(), if the argument plot was True, a figure would be created for each alignment step. This PR gives the option to pass a value of 'reuse' to plot, which well then make it reuse a single plot for each frame. This effectively allows the live monitoring of the alignment process, but without creating more than one figure (i.e. only the final frame is accessible after alignment finishes). This is useful when you have a large alignment job that needs to be monitored (e.g. to watch for divergence), but where its unfeasible with hundreds or thousands of figures. As an alternative, you can also pass a matplotlib.figure.Figure as plot, which will then be used, but I'm not sure if this has any advantages. It just simply follows as a "free" alternative because of how the code is structured.
vidartf
force-pushed
the
ENH_alignment_plots
branch
from
0e2340f
to
d55513f
Compare
|
I nuked and redid on moved files. Needs review. |
| if full_plot: | ||
| axarr[0].set_title('Reference') | ||
| axarr[1].set_title('Image') | ||
| axarr[2].set_title('Phase correlation') |
There was a problem hiding this comment.
The title should be "correlation" when normalize_corr is False.
There was a problem hiding this comment.
This is simply the existing behavior, and as such not really part of this PR :)
|
I still think that the proper way to fix this is by storing or the correlations in a signal and returning it, maybe with the option to plot it as the calculation progresses. What do you think? |
|
@francisco-dlp I think what you are proposing is a different feature than what this PR is attempting to do, and that is can be implemented independently from this. Building up a correlation signal will double the memory use, and as such, plotting without creating this signal would still be useful. |
|
if storing the correlation signal does become a thing, it needs to be optional because of the memory issue - in any case these are certainly separate issues. |
|
I agree. However, it'll be still good to use HyperSpy's image plotting features that automatically use the blitting feature to speep up the update of the image. This is as simple as putting the image data into a |
|
This branch was automatically closed when we renamed |
Previously for
Signal.estimate_shift2D(), if the argumentplotwasTrue, a figure would be created for each alignment step. This PR gives the option to pass a value of'reuse'toplot, which well then make it reuse a single plot for each frame. This effectively allows the live monitoring of the alignment process, but without creating more than one figure (i.e. only the final frame is accessible after alignment finishes). This is useful when you have a large alignment job that needs to be monitored (e.g. to watch for divergence), but where its unfeasible with hundreds or thousands of figures.As an alternative, you can also pass a
matplotlib.figure.Figureasplot, which will then be used, but I'm not sure if this has any advantages. It just simply follows as a "free" alternative because of how the code is structured.