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

[FEAT] Imviz: Rotate image #1466

Closed
pllim opened this issue Jul 7, 2022 · 18 comments
Closed

[FEAT] Imviz: Rotate image #1466

pllim opened this issue Jul 7, 2022 · 18 comments
Labels
feature Feature request imviz

Comments

@pllim
Copy link
Contributor

pllim commented Jul 7, 2022

Description

User wants to rotate an image in several ways:

  1. N-up E-left
  2. Arbitrary angle

Out of scope: Handling distortion accurately.

Current status: @astrofrog is going to present a proof-of-concept on glue-side by 2022-07-12

cc @orifox and @kecnry

Additional context

Previous failed attempts:

🐱

@pllim pllim added feature Feature request imviz labels Jul 7, 2022
This was referenced Jul 7, 2022
@astrofrog
Copy link
Collaborator

astrofrog commented Jul 12, 2022

I've been experimenting with an approach that I think could work - basically internally in glue we would have the ability to apply an arbitrary 2D affine transform to the 'viewport'. To demonstrate this, I have the following PRs:

If you install these two then in the Imviz example notebook you can do:

from matplotlib.transforms import Affine2D
affine_transform = Affine2D().rotate_deg(30)
imviz.app.get_viewer_by_id('imviz-0').state.affine_matrix = affine_transform

and the image will then be rotated. Selections will still work fine. In fact you should in principle be able to do for example:

affine_transform = Affine2D().rotate_deg_around(2048, 1024, 30)

which includes a translation component in the affine transform but this doesn't seem to work properly at the moment in the sense that selections are a bit offset compared to where they are drawn (but I think it should be easy to fix). Ignore for now the fact that this involves Matplotlib transforms - this is just a hack for quick development but won't be in the final code

Does this seem reasonable? Can someone try it out to let me know if it would be fast enough? @pllim if you had already made a plugin to set the rotation you could try and hook it up to this?

I decided on this approach as opposed to adding/removing virtual Data objects as that gets a lot more complicated and updating the link graph etc can be slow. If the approach here works I think it should be a lot faster.

Another caveat is that for now the mouseover coordinates are wrong but that will be an easy fix if we agree on this approach.

@pllim

This comment was marked as resolved.

@pllim
Copy link
Contributor Author

pllim commented Aug 8, 2022

See #1551

@pllim
Copy link
Contributor Author

pllim commented Jan 23, 2024

Done in #2179

@pllim pllim closed this as completed Jan 23, 2024
@stscijgbot-jwql
Copy link

This issue is tracked on JIRA as JDAT-2574.

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

Here's an update. I found some code that should be able to align this and create a new fits file with the correct padding.
import matplotlib.pyplot as plt
from astropy.io import fits
from astropy.wcs import WCS
from reproject import reproject_interp
from reproject.mosaicking import find_optimal_celestial_wcs

Import Data

image = fits.open('iebc16030_drc.fits')

Find WCS where North is pointing up

wcs_in = WCS(image[1].header)
data = image[1].data
wcs_out, shape_out = find_optimal_celestial_wcs([image[1]])

Reproject image

reprojected, foot = reproject_interp((data, wcs_in), wcs_out, shape_out)

plt.imshow(reprojected, origin = 'lower', vmin = -0.038, vmax = 0.098)
plt.grid(color='white', ls=':', alpha=0.7)
plt.xlabel('RA')
plt.ylabel('DEC')
plt.show()

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

!image-2022-12-05-11-18-20-033.png!

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

Sure, you can do that outside of Jdaviz all you want. I would not recommend doing this every time someone click "change angle" inside Jdaviz. This is a very expensive operation.

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

I spoke with Tom yesterday after the demo and mentioned that before we move forward, I would like to see the following:
Reproject (pix_to_pix) applied to the entire array every time someone clicks change angle. A user will typically only want to do this once per image. Tom should test it on a 1 GB and 5 GB image and report the times.

Right now, our cube model fitting takes 1-2 minutes. So I can't imagine it taking longer than that, especially for an average size array.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

What about memory usage? Imagine loading a 10k by 10k Roman image. And then rotate it. What if you are interested in an object that covers 100 pixels? Do you really want to rotate that whole thing or just what you are seeing?

Rotating the entire array means reading everything into buffer, defeating the purpose of memory mapping.

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

Well we may need to consider Roman separately. I can tell you from experience that very few people will want to rotate over just 100 pixels. The most common use case by far (and I've already seen this a dozen times) is someone loads their image into Imviz and wants to rotate it to the normal directions and then start playing with it. 

While I don't completely understand the buffer, I would argue that the new image be treated just like a second loaded image. Imviz should always keep a copy of the original and the newly rotated image. Ideally someone isn't rotating an image 20 times (or even 3 times). But if they do, the newly rotated image will replace the old one unless they hit save or something. In other words, this method is really no different than what I outlined in my code above, except its happening in Jdaviz...and it's happening only once per image (because from what I've seen, that's the most any user wants to do it). We can certainly think of more complex use cases, but this is the basic one that we need to solve ASAP.

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

Regarding the Roman image, one additional thought. Dealing with a 10K by 10K image is going to be a pain in our necks no matter how you slice it. I don't think the reproject is the limiting problem here. We're going to need to brainstorm on those images. But right now, our focus is JWST.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

Even for JWST, there is the consideration for MAST. Don't they deploy on AWS? Do they have EC2 with memory large enough to support this feature? Or do you expect this feature to not be used at all by MAST?

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

We can certainly discuss that possibility. As I mentioned before, model fitting on a cube takes 2-3 minutes. And generates an entiresly new cube. So if we're faster than that...or even on par...I'm totally fine. But happy to talk to the MAST people about specifics.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

If we're trying to compete with DS9 performance, taking 2-3 minutes to rotate one image would also be unacceptable to me.

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

I think we've given up competing with ds9 a long time ago. Reprojecting is not the same thing as rotating anyway. I think we've learned that rotation is not possible at the current time, although as I mentioned yesterday, i do want to revisit it. Anyway, I doubt the reproject will take 2-3 minutes. I run it all the time in the notebook on large files and it takes 5 seconds max. So, my point is, this is an academic conversation until I see how long it actually takes in Imviz once Tom implements it for a 1 and 5 GB image. We need some actual numbers.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

So am I correct in saying that your requested feature is no longer "rotating image" but rather you want a plugin to run the reproject package?

@stscijgbot-jwql
Copy link

Comment by Ori Fox on JIRA:

I'm flexible with the solution that the devs come up with. The requirement is for a user to be able to rotate an image to the correct orientation. I don't care how that happens, but after a number of attempts and ideas, it seems like the reproject package is the optimal way to do it. I agree this isn't a pure rotation, but from my understanding, a pure rotation is more difficult. So I'm happy to stick with this for now.

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

Successfully merging a pull request may close this issue.

3 participants