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

Investigate options to make ArrayAnimatorWCS more efficient when updating WCSAxes #3

Open
Cadair opened this issue Jun 11, 2020 · 1 comment
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Cadair
Copy link
Member

Cadair commented Jun 11, 2020

Currently when using ArrayAnimatorWCS on each iteration of the animator (array slice change) WCSAxes' reset_wcs method is called. reset_wcs makes no assumptions about continuity of the wcs, frame_class or slices arguments between calls. (It is also used as a way to initialise the axes in the first place, where it would be illogical for these assumptions to be made).

This lack of assumptions in reset_wcs is understandable, and very baked into the way the axes are setup, however, in ArrayAnimatorWCS we can make a lot more assumptions about the changes we are making to the plot, because we control the iteration. We know that each re-draw of the plot will only have changed the slice in a way that changes the numerical indices of the one of the sliders. This means that given a slices=['x', 'y', 0] argument to WCSAxes for the first frame, we know that the only thing that's changing in future calls to reset_wcs is the numerical value in that list, the 0.

If we were able to make these assumptions in WCSAxes the only element of the plot we would need to update is the generated transform object (Which is copied to a lot of places in the WCSAxes structure). As this is the only component of the stack which takes into account the numerical values of the slices, all the rest handle setting up the axes, spines, ticks etc and working out what axes are visible. It would be interesting to investigate if it would be possible for ArrayAnimatorWCS to be able to reach into WCSAxes and update the transform without needing to update everything else, and making any changes upstream to facilitate this.

Any changes along these lines would likely lead to some significant performance improvements in the draw speed of the animator, due to a substantially shorter code path on the WCSAxes side. However, without doing some more detailed profiling of the code, I would not recommend doing this just for performance reasons.

The main motivation for this missive is that bypassing the full effects of reset_wcs would lead to a much higher degree of coherence between re-draws in the animator. The most user facing of these changes would be that the existing WCSAxes API could be used to set preferences such as coordinate units or labels. Currently, ArrayAnimatorWCS takes a coord_param dictionary and re-applies the supported settings after every call to reset_wcs. This means that we have created a proxy API for an API that hopefully people are familiar with for their 1D / 2D WCSAxes plots. As discussed in sunpy/sunpy#4270 being able to use the existing API directly would be a great benefit. Implementing that functionality by bypassing large sections of reset_wcs would be the cleanest way, and likely bring other benefits beyond API simplicity.

@dstansby

This comment was marked as outdated.

@Cadair Cadair transferred this issue from sunpy/sunpy Oct 29, 2021
@Cadair Cadair added enhancement New feature or request help wanted Extra attention is needed labels Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants