clawpack / visclaw

Clawpack visualization tools
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
29 stars 47 forks source link

Skip patches not seen #262

Closed rjleveque closed 4 years ago

rjleveque commented 4 years ago

When looping over patches this version skips a patch if it has no intersection with the region specified by plotaxes.xlimits and plotaxes.ylimits. This speeds up plotting substantially for some problems, if you want to zoom in on different regions in different plots and have thousands of patches that can be ignored in each case. This is distinct from what's proposed in #246 but useful in some contexts.

Note a possible problem is that if a user creates a figure with specified xlimits and ylimits but then gives additional xlim, ylim, or axis commands to zoom out, then some patches will not be seen that should be seen in the wider view, which could be misleading or confusing. I'm ok with this but others may differ.

mandli commented 4 years ago

This is great!

mjberger commented 4 years ago

So if you zoom out, what do you have to do for “real” re-plotting that includes the missing patches?

Does is also skip patches for batch?

On Apr 11, 2020, at 6:34 PM, Randall J. LeVeque notifications@github.com wrote:

When looping over patches this version skips a patch if it has no intersection with the region specified by plotaxes.xlimits and plotaxes.ylimits. This speeds up plotting substantially for some problems, if you want to zoom in on different regions in different plots and have thousands of patches that can be ignored in each case. This is distinct from what's proposed in #246 https://github.com/clawpack/visclaw/issues/246 but useful in some contexts.

Note a possible problem is that if a user creates a figure with specified xlimits and ylimits but then gives additional xlim, ylim, or axis commands to zoom out, then some patches will not be seen that should be seen in the wider view, which could be misleading or confusing. I'm ok with this but others may differ.

You can view, comment on, or merge this pull request online at:

https://github.com/clawpack/visclaw/pull/262 https://github.com/clawpack/visclaw/pull/262 Commit Summary

skip plotting patches outside specified axis to speed up plotting add a possible warning message when skipping pastches File Changes

M src/python/visclaw/frametools.py https://github.com/clawpack/visclaw/pull/262/files#diff-99ddcf14ee380b92bc54d3bf33fca376 (26) Patch Links:

https://github.com/clawpack/visclaw/pull/262.patch https://github.com/clawpack/visclaw/pull/262.patch https://github.com/clawpack/visclaw/pull/262.diff https://github.com/clawpack/visclaw/pull/262.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/clawpack/visclaw/pull/262, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGUGCYSYPK7BSXK3ISPTKLRMDWBDANCNFSM4MGFOFOQ.

mandli commented 4 years ago

I am not sure this would make sense for interactive plotting given that a user could move the view into the plot but the xlimits and ylimits would not change. That may be something to add (check for interactive plotting). This should work fine for batch plotting.

rjleveque commented 4 years ago

Yes, it should work fine for batch processing, also in parallel.

When zooming out on a plot there's no easy way to recognize that and now plot the missing patches. So the use case I had in mind was where the appropriate xlimits and ylimits are set in setplot.py for each PlotAxes object, and if you want multiple views at different zooms the user would have to set up multiple plot figures in setplot.py. And note that if you are doing batch processing, i.e. using make plots to make the html pages rather than Iplotclaw for interactive plotting, then there is no way to zoom out other than specifying another figure, so this approach is ideal for batch processing to avoid spending time plotting patches not visible. (Though what is suggested in #246 could be even faster in some contexts.)

For exploratory interactive work using Iplotclaw it is sometimes convenient to be able to zoom out and see all the patches. So maybe this should be modified to somehow allow the user to specify this. For example, we could add a new attribute to ClawPlotAxes so that the user could specify (in setplot),

    plotaxes.plot_all_patches = True

to revert to the old behavior, whereas by default this attribute would be False and so existing setplot files would lead to the new behavior. Or maybe a better name would be plot_patches_outside_extent or something else.

What do you think?

mjberger commented 4 years ago

I think that \'s good idea (a new plot-all-patches variables to remind user of the new behavior as well as how to revert).

On Apr 13, 2020, at 12:10 PM, Randall J. LeVeque notifications@github.com wrote:

Yes, it should work fine for batch processing, also in parallel.

When zooming out on a plot there's no easy way to recognize that and now plot the missing patches. So the use case I had in mind was where the appropriate xlimits and ylimits are set in setplot.py for each PlotAxes object, and if you want multiple views at different zooms the user would have to set up multiple plot figures in setplot.py. And note that if you are doing batch processing, i.e. using make plots to make the html pages rather than Iplotclaw for interactive plotting, then there is no way to zoom out other than specifying another figure, so this approach is ideal for batch processing to avoid spending time plotting patches not visible. (Though what is suggested in #246 https://github.com/clawpack/visclaw/issues/246 could be even faster in some contexts.)

For exploratory interactive work using Iplotclaw it is sometimes convenient to be able to zoom out and see all the patches. So maybe this should be modified to somehow allow the user to specify this. For example, we could add a new attribute to ClawPlotAxes so that the user could specify (in setplot),

plotaxes.plot_all_patches = True

to revert to the old behavior, whereas by default this attribute would be False and so existing setplot files would lead to the new behavior. Or maybe a better name would be plot_patches_outside_extent or something else.

What do you think?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/clawpack/visclaw/pull/262#issuecomment-612966311, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGUGC5G4GPOCPXP3HWWV7DRMM2PHANCNFSM4MGFOFOQ.