Open pllim opened 1 year ago
I want to hide in a corner...
Is there something get_interactive_regions
does that get_subsets
does not? Or does better?
Re: https://github.com/spacetelescope/jdaviz/issues/2230#issuecomment-1572040691
@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.
If this is too much reading, jump to "tl;dr" below.
Case 1
Use bmorris3/jdaviz#5 and
notebooks/concepts/imviz_rotation_by_hidden_layer.ipynb
. Stop after these cells:Blink to "A" (the original reference data that is no longer reference data). Draw a rectangle.
Keep in mind that this is an image with the shape
(ny, nx)
of(10, 8)
. Now get the Subset back out asregions
shape using our existing API: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:
To get a more realistic rectangle for the image, we actually have to do this:
Then you will see:
But is this truly correct? Continue on to Case 2.
Case 2a
Run
notebooks/ImvizDitheredExample.ipynb
. You can stop right afterimviz.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).
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:
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 https://github.com/bmorris3/jdaviz/pull/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):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:
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.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:At this point, if you have not fainted, you should see something like this:
Now run
imviz.get_interactive_regions()
: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.https://github.com/spacetelescope/jdaviz/blob/d86fd9a792f8abdb1062fe606fbbb4322f2cbab7/jdaviz/core/helpers.py#L805-L806
😱 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
🐱 🐱