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

Subset behavior after changing reference data in WCS linking mode #2230

Open
pllim opened this issue May 31, 2023 · 3 comments
Open

Subset behavior after changing reference data in WCS linking mode #2230

pllim opened this issue May 31, 2023 · 3 comments

Comments

@pllim
Copy link
Contributor

pllim commented May 31, 2023

If this is too much reading, jump to "tl;dr" below.

Case 1

Use bmorris3#5 and notebooks/concepts/imviz_rotation_by_hidden_layer.ipynb. Stop after these cells:

imviz.app._change_reference_data(imviz.app._wcs_only_label)
print(imviz.default_viewer.state.reference_data.label)
imviz.link_data(link_type="wcs")
imviz.default_viewer.zoom_level = "fit"

Blink to "A" (the original reference data that is no longer reference data). Draw a rectangle.

Screenshot 2023-05-31 165030

Keep in mind that this is an image with the shape (ny, nx) of (10, 8). Now get the Subset back out as regions shape using our existing API:

>>> imviz.get_interactive_regions()

Instead of a rectangle roughly 5 x 3 pixels in dimension (though strictly speaking, it isn't even a rectangle anymore), you will see an unrealistically large rectangle like this centered way off the image:

RectanglePixelRegion(
    center=PixCoord(x=877.1443472960048, y=-347.4749125945692),
    width=2901.591077706634,
    height=1259.4139996854321,
    angle=0.0 rad)

To get a more realistic rectangle for the image, we actually have to do this:

subset_1 = imviz.get_interactive_regions()['Subset 1']
wcs_ref = imviz.default_viewer.state.reference_data.coords
wcs_a = imviz.app.data_collection[0].coords
subset_1.to_sky(wcs_ref).to_pixel(wcs_a)

Then you will see:

RectanglePixelRegion(
    center=PixCoord(x=3.1506652284308414, y=4.522711118809012),
    width=5.648188716271707,
    height=2.451554251317931,
    angle=-0.4784845394926116 rad)

But is this truly correct? Continue on to Case 2.

Case 2a

Run notebooks/ImvizDitheredExample.ipynb. You can stop right after imviz.show() and then link them by WCS via the plugin (or skip down the notebook and run the API, up to you).

Zoom in on an obvious object (e.g., a bright star). Draw a circle around it. When you blink, both the mouseover and the Subset displays are smart enough to show you something sane (i.e., pixel values jump but not the sky coordinates if you keep your mouse steady over the same pixel, and Subset stays put).

Screenshot 2023-05-31 170257

Screenshot 2023-05-31 170321

At this point, A is the reference image and this is what you will see from the example above when you grab the Subset using same API as Case 1 above:

CirclePixelRegion(
    center=PixCoord(x=1799.91748046875, y=1142.35693359375),
    radius=25.42364501953125)

Now, change the reference to B (a handy way at this point is to use the DATA menu click as shown in the video of bmorris3#5). After encountering Case 1, what would you expect to see when you run imviz.get_interactive_regions()? Would the circle now jump a few pixels because B is now the reference?

Wrong. It stays exactly the same as if A is still the reference image. This is due to:

This time, we have to do the opposite of Case 1 to get the correct CirclePixelRegion for B (now the reference data):

subset_1 = imviz.get_interactive_regions()['Subset 1']
wcs_ref = imviz.default_viewer.state.reference_data.coords
wcs_a = imviz.app.data_collection[0].coords
subset_1.to_sky(wcs_a).to_pixel(wcs_ref)
CirclePixelRegion(
    center=PixCoord(x=1813.7413872365673, y=1156.3583226415858),
    radius=25.41803559679621)

Why the difference here? Is there some special logic we're doing Subsets in the presence of WCS-only that does not apply when it does not exist?

Case 2b

Still on the same session as Case 2a above, now we run some new code:

from jdaviz.configs.imviz.wcs_utils import _get_rotated_nddata_from_label

data = imviz.app.data_collection[1]  # acs_47tuc_2[SCI,1]
degn = 45 * u.deg
fake_ndd_rotated = _get_rotated_nddata_from_label(imviz.app, data.label, degn)

imviz.load_data(fake_ndd_rotated, data_label=imviz.app._wcs_only_label)

You will see the WCS-only layer in the DATA menu. Select it as the new reference data (using the GUI or imviz.app._change_reference_data(imviz.app._wcs_only_label) makes no difference).

This next behavior might be machine dependent, but now I see some lag when I pan zoom and I can hear the CPU churning away when I do it. Are we already hitting the performance limit with 3 images (one of them hidden) and 1 subset?

What will imviz.get_interactive_regions()['Subset 1'] show now? Still the same as Case 2a! Well, that ruled out the presence of WCS-only layer changes Subset retrieval behavior.

<CirclePixelRegion(
    center=PixCoord(x=1799.91748046875, y=1142.35693359375),
    radius=25.42364501953125)>

Case 3

Why is imviz.get_interactive_regions() behavior inconsistent between Case 1 and Case 2? To find out, let's rerun Case 1 above with the following variation:

  • This time, load the WCS-only image but do not make it a reference yet.
  • Make sure they are linked by WCS.
  • Draw a rectangle while A is still the reference image.
  • Now change the reference data to the WCS-only layer.
  • Draw a second rectangle.
  • Now change the reference data to B (second loaded real data).
  • Draw a third rectangle.

At this point, if you have not fainted, you should see something like this:

Screenshot 2023-05-31 175609

Now run imviz.get_interactive_regions():

{'Subset 1': <RectanglePixelRegion(
    center=PixCoord(x=4.0151315785549535, y=4.471710525839782),
    width=5.0526315792037515, height=2.0000000001014846, angle=0.0 rad)>,
 'Subset 2': <RectanglePixelRegion(
    center=PixCoord(x=640.2807306681996, y=-335.12771651922185),
    width=2963.327058083372, height=938.3869017264005, angle=0.0 rad)>,
 'Subset 3': <RectanglePixelRegion(
    center=PixCoord(x=3.85723684185211, y=4.019078947052538),
    width=6.210526315099738, height=2.9894736838785176, angle=0.0 rad)>}

Is something dawning on you now? Are you trembling with fear? Well, I did. Proceed to the next section.

tl;dr

😱 imviz.get_interactive_regions() that really just calls this below under the hood needs its logic fixed to account for which data was reference when it was drawn. It is not always the same data nor the current data.

region = subset_data.data.get_selection_definition(
subset_id=subset_label, format='astropy-regions')

😱 To accurate grab the regions shape for each image when linked by WCS, we must use sky regions. This also means that we should disallow mixing pixel and WCS links in the same session, which means some of the currently supported use cases (is anyone even using them?) will no longer work.

😱 Performance could already be an issue for some people when running Case 2b above.

Also see

🐱 🐱

@camipacifici
Copy link
Contributor

I want to hide in a corner...

@javerbukh
Copy link
Contributor

Is there something get_interactive_regions does that get_subsets does not? Or does better?

@pllim
Copy link
Contributor Author

pllim commented Jun 1, 2023

Re: #2230 (comment)

@javerbukh , I think the former existed way before the latter? I didn't test this with get_subsets since I wasn't here most of the time when it was developed, if it is what I think it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants