Imviz: Region creation lag for large/linked images #723

Jdaviz-Triage-Bot · 2021-07-08T15:44:03Z

Reporter: @pllim

~~Suspected to be caused by glue-viz/glue-jupyter#241 (unconfirmed). Also see glue-viz/glue-jupyter#241 (comment)~~

~~Initial investigation needed to pin down the cause before further work can be done.~~

Blocked by WCS performance issues in linked data, see Imviz: Region creation lag for large/linked images #723 (comment)

To reproduce:

Fire up Imviz.
Load two large images, say, 2k by 4k. Make sure they have WCS. Link them. (Or run the Imviz example notebook.)
Interactively draw a region.
Move the region to a different spot.
See lag (both red and gray regions appear, can be confusing); a chance to go get some coffee ☕ .

DISCLAIMER: This issue was autocreated by the Jdaviz Issue Creation Bot on behalf of the reporter. If any information is incorrect, please contact Duy Nguyen

pllim · 2021-07-08T16:53:12Z

Okay, I think it was unfair to blame glue-viz/glue-jupyter#241 . Turns out it is a performance problem that showed up when I changed the example images from ones with dimensions (720, 721) and (149, 149) to actual HST ACS/WFC exposures with dimensions both (2048, 4096). 👀

Might be related to glue-viz/glue#2138

What I can do here is to find some smaller images as examples (#724) but that is not the real solution, as users will still notice the lag with real observations.

rosteen · 2021-07-14T14:20:28Z

Just chiming in with a +1 for this not being fixed by using smaller images. since we want it to work with real data. I'll also note that I'm seeing lag not just when dragging the regions around, but also when panning/zooming the image once regions are added.

pllim · 2021-07-14T18:24:19Z

@rosteen has since narrowed this down to WCS operations in linked images + Subset. He will provide @astrofrog with the profiling info for advise on the proper fix.

Another "hack" we can do is to never link the images in Imviz, but it might not be always possible, we have to refactor blinking, and I am not sure how Subset work in un-linked data.

pllim · 2021-07-14T18:26:17Z

Also @ibusko has noted that this lag isn't as obvious on his machine. 🤷

pllim · 2021-07-16T15:31:47Z

Another idea: When images are loaded only link by pixels. No lag there and Blink works. You can still match them by WCS using the "pan/zoom by WCS" (crossed arrows with links) button. Downside of that is when you only have one viewer and you blink, you can see the object in dithered images jump around, but you also see that in DS9, so no big deal?

from glue.core.component_link import ComponentLink

dc = imviz.app.data_collection
x_link = ComponentLink([dc[0].id['Pixel Axis 1 [x]']], dc[1].id['Pixel Axis 1 [x]'])
y_link = ComponentLink([dc[0].id['Pixel Axis 0 [y]']], dc[1].id['Pixel Axis 0 [y]'])
dc.add_link(x_link)
dc.add_link(y_link)

See #735

pllim · 2021-07-16T18:38:47Z

Took at stab looking into performance on WCS linking + subset. I agree with @rosteen 's assessment that it is caused by calling wcs_autolinking.py. That also explains why you don't see it when WCS is not involved. Alas, I don't know how to fix this problem still. All I can do is document my findings here.

Change this in glue

Apply this patch to your installed copy of glue/plugins/coordinate_helpers/link_helpers.py:

--- a/glue/plugins/wcs_autolinking/wcs_autolinking.py
+++ b/glue/plugins/wcs_autolinking/wcs_autolinking.py
@@ -1,5 +1,6 @@
 import copy
 
+import numpy as np
 from astropy.wcs import WCS
 from astropy.wcs.utils import pixel_to_pixel
 from astropy.wcs.wcsapi import SlicedLowLevelWCS, HighLevelWCSWrapper
@@ -17,9 +18,11 @@ class IncompatibleWCS(Exception):
 def get_cids_and_functions(wcs1, wcs2, pixel_cids1, pixel_cids2):
 
     def forwards(*pixel_input):
+        print(f'forwards pixel_input: {pixel_input if np.isscalar(pixel_input[0]) else pixel_input[0].shape}')
         return pixel_to_pixel(wcs1, wcs2, *pixel_input)
 
     def backwards(*pixel_input):
+        print(f'backwards pixel_input: {pixel_input if np.isscalar(pixel_input[0]) else pixel_input[0].shape}')
         return pixel_to_pixel(wcs2, wcs1, *pixel_input)
 
     pixel_input = [0] * len(pixel_cids1)

Run these in a notebook

Most of these are some cells from the Imviz example notebook.

import warnings
warnings.simplefilter('ignore')

import matplotlib.pyplot as plt
import numpy as np
from astropy import units as u
from astropy.coordinates import SkyCoord, Angle
from astropy.table import Table
from astropy.utils.data import download_file
from glue.plugins.wcs_autolinking.wcs_autolinking import wcs_autolink, WCSLink
from photutils import CircularAperture, SkyCircularAperture
from regions import PixCoord, CirclePixelRegion, CircleSkyRegion

from jdaviz import Imviz

acs_47tuc_1 = download_file('https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:HST/product/jbqf03gjq_flc.fits', cache=True)
acs_47tuc_2 = download_file('https://mast.stsci.edu/api/v0.1/Download/file?uri=mast:HST/product/jbqf03h1q_flc.fits', cache=True)

imviz = Imviz()
imviz.load_data(acs_47tuc_1, data_label='acs_47tuc_1')
imviz.load_data(acs_47tuc_2, data_label='acs_47tuc_2')

viewer = imviz.app.get_viewer('viewer-1')

# Manually link the data. We can remove this when Imviz auto-linking issue is resolved.
# This is necessary for blink to function properly.
wcs_links = wcs_autolink(viewer.session.data_collection)
for link in wcs_links:
    exists = False
    for existing_link in viewer.session.data_collection.external_links:
        if isinstance(existing_link, WCSLink):
            if (link.data1 is existing_link.data1
                    and link.data2 is existing_link.data2):
                exists = True
                break
    # Add only those links that don't already exist
    if not exists:
        viewer.session.data_collection.add_link(link)

# Because linking happens after load, the image display is broken a little.
# So, please do this manually **after** running this cell.
#
# 1. Uncheck both data from Data menu.
# 2. Re-check both data from Data menu.

imviz.app

my_reg = CirclePixelRegion(center=PixCoord(x=600, y=200), radius=20)

imviz.load_static_regions({'my_reg': my_reg})

Result

# linked images	# subset	# forwards/backwards calls
1	1	0
2	1	8
3	1	16
4	1	24

This maps to (num_linked_images - num_subset) * 2**3 (at least when num_subset = 1). Example print output for 4 linked images (each image total dimension is 4096x2048).

>>> imviz.load_static_regions({'my_reg': my_reg})
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
forwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)
backwards pixel_input: (462, 1703)

pllim · 2021-07-27T19:32:26Z

At Imviz Hack Hour, user expressed concern on various performance issues. These might be deal breaker for switching away from DS9 to Jdaviz.

astrofrog · 2021-08-03T14:10:29Z

One step towards improving performance with the linking is to avoid linking all datasets pairwise but instead choosing one as a reference and linking other ones to it. If we actually do this, we should make sure we update the link tree when datasets are added/removed though, especially if the reference one is removed.

pllim · 2021-08-03T14:14:45Z

@astrofrog , this is what I have to work with right now in the Imviz notebook. It would be nice if you can provide a similar example of what you mean in #723 (comment)

# Manually link the data. We can remove this when Imviz auto-linking issue is resolved.
# This is necessary for blink to function properly.
wcs_links = wcs_autolink(viewer.session.data_collection)
for link in wcs_links:
    exists = False
    for existing_link in viewer.session.data_collection.external_links:
        if isinstance(existing_link, WCSLink):
            if (link.data1 is existing_link.data1
                    and link.data2 is existing_link.data2):
                exists = True
                break
    # Add only those links that don't already exist
    if not exists:
        viewer.session.data_collection.add_link(link)

pllim · 2021-08-03T14:16:23Z

Also, the lag goes beyond dataset. It also gets worse when Subsets are added to linked data; is your proposed solution also going to fix this?

pllim · 2021-08-03T17:33:35Z

From Tom R: the links returned by wcs_autolink each have data1 and data2, you can just decide which ones to add to the data collection... you just need to make sure we don't add all links

pllim · 2021-08-03T18:35:08Z

So, I don't think the code in the Imviz example notebook is the problem. In that example, only two images are added and wcs_autolink only returns a list with one element.

>>> wcs_links = wcs_autolink(viewer.session.data_collection)
>>> wcs_links
[<glue.plugins.wcs_autolinking.wcs_autolinking.WCSLink at ...>]
>>> viewer.session.data_collection.external_links
(<glue.core.link_helpers.LinkSame at ...>,)

We have no control of how Subset is linked to the data there. Will have to dig more.

pllim · 2021-08-03T19:57:16Z

One step towards improving performance with the linking is to avoid linking all datasets pairwise but instead choosing one as a reference and linking other ones to it.

@astrofrog , I went into several rabbit holes in glue-jupyter and glue-core but got lost among all the session and hub stuff. I cannot see any obvious/easy way to do this for Subsets.

pllim · 2021-08-05T14:23:48Z

@astrofrog thinks this is caused by not just any WCS, but by distortion in the WCS. The two ACS images in the Imviz example notebook (jbqf03gjq_flc.fits and jbqf03h1q_flc.fits) are not yet corrected for distortion and they are dithered by one pixel or so. He thinks this can be solved by smarter linking in wcs_autolink but he needs to "think about it."

pllim · 2021-08-05T15:54:32Z

@astrofrog thinks the linking could be simplified over at glue-core to use simple affine transforms, hence increasing the performance. He will have a proof-of-concept by 2021-08-10.

pllim added imviz bug Something isn't working Upstream fix required labels Jul 8, 2021

pllim self-assigned this Jul 8, 2021

pllim mentioned this issue Jul 8, 2021

Add ellipse mode glue-viz/glue-jupyter#241

Merged

pllim changed the title ~~Imviz: Region creation lag~~ Imviz: Region creation lag for large image Jul 8, 2021

pllim mentioned this issue Jul 8, 2021

Use smaller images for Imviz example notebook #724

Closed

pllim removed their assignment Jul 8, 2021

pllim changed the title ~~Imviz: Region creation lag for large image~~ Imviz: Region creation lag for large/linked images Jul 14, 2021

pllim mentioned this issue Jul 15, 2021

Imviz: Subset control plugin #729

Closed

3 tasks

pllim mentioned this issue Jul 16, 2021

Imviz: Only link by pixel #735

Merged

6 tasks

pllim added performance Performance related 🔥 Critical labels Jul 28, 2021

pllim mentioned this issue Jul 28, 2021

BUG: Region creation is laggy #712

Closed

pllim added this to the Imviz 1.0 milestone Jul 28, 2021

pllim mentioned this issue Jul 29, 2021

JDAT-1401: Benchmark against DS9 #524

Open

pllim assigned astrofrog Aug 5, 2021

This was referenced Aug 5, 2021

Significantly speed up operations that rely on WCS links #765

Merged

BUG: Preserve Imviz zoom level when adding markers (brute force) #767

Closed

pllim closed this as completed in #765 Aug 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Imviz: Region creation lag for large/linked images #723

Imviz: Region creation lag for large/linked images #723

Jdaviz-Triage-Bot commented Jul 8, 2021 •

edited by pllim

Loading

pllim commented Jul 8, 2021 •

edited

Loading

rosteen commented Jul 14, 2021

pllim commented Jul 14, 2021

pllim commented Jul 14, 2021

pllim commented Jul 16, 2021 •

edited

Loading

pllim commented Jul 16, 2021 •

edited

Loading

pllim commented Jul 27, 2021

astrofrog commented Aug 3, 2021

pllim commented Aug 3, 2021 •

edited

Loading

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 5, 2021 •

edited

Loading

pllim commented Aug 5, 2021

Imviz: Region creation lag for large/linked images #723

Imviz: Region creation lag for large/linked images #723

Comments

Jdaviz-Triage-Bot commented Jul 8, 2021 • edited by pllim Loading

pllim commented Jul 8, 2021 • edited Loading

rosteen commented Jul 14, 2021

pllim commented Jul 14, 2021

pllim commented Jul 14, 2021

pllim commented Jul 16, 2021 • edited Loading

pllim commented Jul 16, 2021 • edited Loading

Change this in glue

Run these in a notebook

Result

pllim commented Jul 27, 2021

astrofrog commented Aug 3, 2021

pllim commented Aug 3, 2021 • edited Loading

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 3, 2021

pllim commented Aug 5, 2021 • edited Loading

pllim commented Aug 5, 2021

Jdaviz-Triage-Bot commented Jul 8, 2021 •

edited by pllim

Loading

pllim commented Jul 8, 2021 •

edited

Loading

pllim commented Jul 16, 2021 •

edited

Loading

pllim commented Jul 16, 2021 •

edited

Loading

pllim commented Aug 3, 2021 •

edited

Loading

pllim commented Aug 5, 2021 •

edited

Loading