Skip to content

ViewerImageProxy: allow data operations on plotted mosaics#812

Merged
ejeschke merged 20 commits intoejeschke:masterfrom
naojsoft:viewer-image-proxy
Apr 24, 2020
Merged

ViewerImageProxy: allow data operations on plotted mosaics#812
ejeschke merged 20 commits intoejeschke:masterfrom
naojsoft:viewer-image-proxy

Conversation

@ejeschke
Copy link
Owner

@ejeschke ejeschke commented Dec 6, 2019

Adds a ViewerImageProxy class, which is a kind of stand-in for an image (i.e. BaseImage-based) wrapper. It looks at images plotted on the canvas and provides SOME methods to treat it as though it was one large single image. This allows us to save large amounts of memory for mosaiced fields,
because we don't need to allocate one huge ndarray to hold everything, but only plot each image at the proper location and orientation on the viewer's canvas--like a sparse data array.

Several plugins are modified so that they will work with this kind of mosaiced display, particularly Pick, Cuts, Histogram, PixTable, Pan.

@ejeschke ejeschke self-assigned this Dec 6, 2019
@ejeschke ejeschke added this to the 3.1 milestone Dec 6, 2019
@ejeschke ejeschke requested a review from pllim December 6, 2019 01:12
@ejeschke
Copy link
Owner Author

ejeschke commented Dec 6, 2019

@pllim, this is preliminary WIP, but far enough along now for testing and some feedback. Hold the code review for now, as there is much to do there, but try out the Mosaic plugin with some tiles (HST?) when you have a chance. When you have a mosaic up, try using Pick/Cuts/Histogram/PixTable.

In a nutshell, what this does is replace the idea of building one large image with all the tiles in it, and instead just plots them on the ginga canvas relative to the WCS. Advantages are that saves memory, can be faster to create the mosaic and doesn't require the user to specify a FOV size. Indeed, the tiles can be quite far apart, as they are simply not plotted unless in the clip area.

The downside: most Ginga plugins have been written with the idea that there is a single image "loaded" into the viewer. Many of them work by trying to get that single image and do something with it. This PR makes a kind of "proxy" that supports some of the image functions; in particular, the functions meant for grabbing small subsets of data and doing some analysis on that. This PR has been written to allow them to make only minor modifications to work with both "regular" images and the mosaics.

@pllim

This comment has been minimized.

@pllim

This comment has been minimized.

@ejeschke ejeschke force-pushed the viewer-image-proxy branch from bb49617 to 82c357a Compare January 8, 2020 04:33
@ejeschke

This comment has been minimized.

@ejeschke

This comment has been minimized.

@pllim

This comment has been minimized.

@ejeschke ejeschke force-pushed the viewer-image-proxy branch from 1173f90 to 0395b84 Compare January 8, 2020 23:45
@ejeschke

This comment has been minimized.

@ejeschke
Copy link
Owner Author

ejeschke commented Jan 9, 2020

Please ping me again when this has been rebased on top of #815.

Ping! 😺

@pllim

This comment has been minimized.

@ejeschke
Copy link
Owner Author

Rebased.

@pllim
Copy link
Collaborator

pllim commented Jan 23, 2020

  1. The non-backward compatibility of Mosaic broke my MosaicAuto plugin downstream.
2020-01-23 13:54:28,438 | E | Control.py:445 (error_wrap) | TypeError
mosaic() got an unexpected keyword argument 'new_mosaic'  File ".../ginga/rv/Control.py", line 435, in error_wrap
    return method(*args, **kwargs)

I guess I'll have to go into https://github.com/spacetelescope/stginga/blob/master/stginga/plugins/MosaicAuto.py and make it check for Ginga version and apply the correct API, but since you said this is WIP and not ready for code review, I'll hold off on that.

  1. When I start ginga with some pre-existing config files in my ~/.ginga, I get the following error but I don't know where it is coming from. However, it only appeared the first two times I ran ginga and then mysteriously disappeared!
2020-01-23 14:00:53,728 | E | Callback.py:172 (_do_callbacks) | Error making callback 'field-info': 'NoneType' object has no attribute 'extdata'
2020-01-23 14:00:53,728 | E | Callback.py:173 (_do_callbacks) | Traceback:
  File ".../ginga/misc/Callback.py", line 156, in _do_callbacks
    res = method(*cb_args, **cb_kwargs)

  File ".../ginga/rv/plugins/Info.py", line 492, in field_info
    if '_info_info' not in channel.extdata:

2020-01-23 14:00:53,728 | E | Callback.py:172 (_do_callbacks) | Error making callback 'field-info': 'NoneType' object has no attribute 'fitsimage'
2020-01-23 14:00:53,728 | E | Callback.py:173 (_do_callbacks) | Traceback:
  File ".../ginga/misc/Callback.py", line 156, in _do_callbacks
    res = method(*cb_args, **cb_kwargs)

  File ".../ginga/rv/plugins/Cursor.py", line 158, in field_info_cb
    fitsimage = channel.fitsimage
  1. The Mosaic plugin from this PR seems to work, but I don't see the mosaic's output buffer in Contents. As a result, there is no way I can write it out via SaveImage, which reads from Contents.

@pllim
Copy link
Collaborator

pllim commented Jan 23, 2020

To clarify, for the Mosaic, I used simulated JWST NIRCam data rather than HST data. They are the three *nrca5_assign_wcs.fits files in Ginga dev Box folder. I am also using Numpy 1.18.1 from conda-forge and dev version of astropy.

  1. Pick failed to find the peak in the resulting mosaic, although as far as my eyes can see, I think the source is pretty decent?

Screenshot from 2020-01-23 14-12-55

2020-01-23 14:11:12,583 | W | AutoCuts.py:173 (calc_histogram) | NaN's found in data, using workaround for histogram
2020-01-23 14:11:12,584 | E | Callback.py:172 (_do_callbacks) | Error making callback 'set': autodetected range of [nan, nan] is not finite
2020-01-23 14:11:12,584 | E | Callback.py:173 (_do_callbacks) | Traceback:
  File "...ginga/misc/Callback.py", line 156, in _do_callbacks
    res = method(*cb_args, **cb_kwargs)

  File ".../ginga/ImageView.py", line 2719, in auto_levels_cb
    self.auto_levels()

  File ".../ginga/ImageView.py", line 2692, in auto_levels
    loval, hival = autocuts.calc_cut_levels(image)

  File ".../ginga/AutoCuts.py", line 153, in calc_cut_levels
    bnch = self.calc_histogram(data, pct=self.pct, numbins=self.numbins)

  File ".../ginga/AutoCuts.py", line 187, in calc_histogram
    density=False)

  File "<__array_function__ internals>", line 6, in histogram

  File ".../numpy/lib/histograms.py", line 795, in histogram
    bin_edges, uniform_bins = _get_bin_edges(a, bins, range, weights)

  File ".../numpy/lib/histograms.py", line 429, in _get_bin_edges
    first_edge, last_edge = _get_outer_edges(a, range)

  File ".../numpy/lib/histograms.py", line 327, in _get_outer_edges
    "autodetected range of [{}, {}] is not finite".format(first_edge, last_edge))

2020-01-23 14:11:12,645 | E | Pick.py:1575 (update_pick) | Error calculating quality metrics: Cannot find bright peaks
2020-01-23 14:11:12,646 | E | Pick.py:1582 (update_pick) |   File ".../ginga/rv/plugins/Pick.py", line 1499, in update_pick
    raise Exception(msg)

@pllim
Copy link
Collaborator

pllim commented Jan 23, 2020

  1. For the Cuts plugin, I drew a line across the mosaic buffer but the plot remains empty. Nothing is plotted and I don't see any errors on the terminal.

  2. Similarly, I also get empty plot for Histogram but there are errors:

2020-01-23 14:19:36,697 | W | AutoCuts.py:173 (calc_histogram) | NaN's found in data, using workaround for histogram
2020-01-23 14:19:36,697 | W | logger.py:174 (_showwarning) | .../ginga/AutoCuts.py:177: RuntimeWarning: All-NaN slice encountered
  minval = np.nanmin(data)

2020-01-23 14:19:36,697 | W | logger.py:174 (_showwarning) | .../ginga/AutoCuts.py:178: RuntimeWarning: All-NaN slice encountered
  maxval = np.nanmax(data)

Error making callback 'draw-event': autodetected range of [nan, nan] is not finite
Traceback:
  File ".../ginga/misc/Callback.py", line 156, in _do_callbacks
    res = method(*cb_args, **cb_kwargs)

  File ".../ginga/rv/plugins/Histogram.py", line 509, in draw_cb
    return self.redo()

  File ".../ginga/rv/plugins/Histogram.py", line 342, in redo
    res = self.histogram_data(data_np, pct=1.0, numbins=numbins)

  File ".../ginga/rv/plugins/Histogram.py", line 321, in histogram_data
    return self.autocuts.calc_histogram(data, pct=pct, numbins=numbins)

  File ".../ginga/AutoCuts.py", line 187, in calc_histogram
    density=False)

  File "<__array_function__ internals>", line 6, in histogram

  File ".../numpy/lib/histograms.py", line 795, in histogram
    bin_edges, uniform_bins = _get_bin_edges(a, bins, range, weights)

  File ".../numpy/lib/histograms.py", line 429, in _get_bin_edges
    first_edge, last_edge = _get_outer_edges(a, range)

  File ".../numpy/lib/histograms.py", line 327, in _get_outer_edges
    "autodetected range of [{}, {}] is not finite".format(first_edge, last_edge))
  1. PixTable shows all nan even when the "Value" in info bar shows actual numbers.

Copy link
Collaborator

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted my findings in the comments above.

I am also concerned about taking away the ability to save the output mosaic. Is there a way to retain the "classic" mosaic plugin so I can still save the mosaic out somehow?

@ejeschke
Copy link
Owner Author

ejeschke commented Jan 23, 2020

@pllim, thanks for testing! I'll get to points 1, 2, and 3, but for now could you update the PR checkout and try again 4, 5, 6, & 7? Seems I messed up this PR when I merged #815, and I just submitted a commit that should make it right.

@ejeschke
Copy link
Owner Author

When I start ginga with some pre-existing config files in my ~/.ginga, I get the following error but I don't know where it is coming from. However, it only appeared the first two times I ran ginga and then mysteriously disappeared!

Has it returned? Does it happen every time you start up ginga from terminal?

@ejeschke
Copy link
Owner Author

The Mosaic plugin from this PR seems to work, but I don't see the mosaic's output buffer in Contents.

We could create Contents and Thumbs entries that would rebuild the mosaic when clicked.

As a result, there is no way I can write it out via SaveImage, which reads from Contents.

This is kind of a limitation of this approach--because no single image is "built", there is no one large image that can be written out as a FITS file.

@ejeschke
Copy link
Owner Author

am also concerned about taking away the ability to save the output mosaic. Is there a way to retain the "classic" mosaic plugin so I can still save the mosaic out somehow?

I'm certainly not opposed to keeping the existing plugin around--we still have uses for that here at Subaru. I think we'd just need to be careful in naming and documentation, so that new users don't get confused between the two plugins.

@ejeschke
Copy link
Owner Author

Existing vs. New plugin

New plugin

  • no need to allocate a large array to hold all the image contents (saving time and potentially a very large amount of memory if there are a lot of images in the mosaic)
  • no need to specify output FOV or worry about it
  • little bit quicker to build mosaics (depends again on constituent images)
  • Complicates operations on "the image" (because there is no one image)
  • Makes it difficult to save the mosaic as a file

Existing plugin

  • Works with existing single image workflows/plugins, because it builds a single image from the constituents
  • Can use lots of memory, depending on size
  • Can be slower, depending on size
  • Must specify FOV for best results
  • Has the problem of making up a value for the space between images (which affects numerical operations on the array--should it be NaN, 0, etc)

@ejeschke
Copy link
Owner Author

#812 (comment) is still an issue but not a blocker (maybe open a follow up issue for it after merge).

Pushed a fix for this.

PixTable still reports the wrong value that does not match what is given on Value.

Can you show an example with FITS for this? I worked really hard to squash that one and was pretty sure I had figured it out. Did it happen when opening a file in the usual way or when using Collage?

@ejeschke
Copy link
Owner Author

I don't see the Travis CI result of this PR. Did it not run or did it simply not reporting back? Can check again when new commits are pushed.

Not sure what happened, but it looks like it ran when I pushed this last fix. Seems ok.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Can you show an example with FITS for this?

Any FITS file will do. I used an example HST/ACS image (name goes like j....fits) and those 2 JWST simulated images above used to create a Collage. Yes, PixTable shows the wrong values for both FITS image loaded directly with Open Files plugin and also wrong for the buffer in Collage. The highlighted number in the middle of PixValue table does not match the number reported by Value when I mouse around the images.

looks like it ran

I only see ReadTheDocs reporting back. That one only builds the documentation for this PR. I do not see any result from TravisCI. Seems like the CI ran but not reporting back -- https://travis-ci.org/github/ejeschke/ginga/pull_requests

@ejeschke
Copy link
Owner Author

Collage and Mosaic do not play well together. That is, when I have a buffer from Mosaic already and then I use Mosaic, the buffer from Mosaic gets messed up. Maybe this should be a warning on the docs of the plugins.

I assume you mean "buffer from Mosaic and then use Collage"?

Unless you press "New Collage" after starting the Collage plugin it will use the loaded image (if any) as the reference for positioning any tiles dragged in afterward. Try making a small mosaic with Mosaic then start Collage and drag in another tile. It should be overplotted in the correct position relative to the others. Here is an example:

collage_after_mosaic

Using Mosaic after Collage does not really make any sense, since Mosaic wants to start by building and loading an empty image with the first thing you drag in. That will displace the tile used by Collage, and erase its other plotted tiles.

I agree that these two plugins really don't play well together and the user should choose one or the other and not mix them. I can add some more documentation on Collage's limitations. Collages for now should be considered kind of ephemeral, since they never exist as whole images in the sense that other images in Ginga do. Maybe another PR can be made that will make entries in the channel for them and make it possible to reconstruct them just by clicking on a thumbnail.

@ejeschke
Copy link
Owner Author

I only see ReadTheDocs reporting back. That one only builds the documentation for this PR. I do not see any result from TravisCI. Seems like the CI ran but not reporting back

Isn't this it?

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Yes, it ran. But I don't see it here.

Untitled

@ejeschke
Copy link
Owner Author

Any FITS file will do. I used an example HST/ACS image (name goes like j....fits) and those 2 JWST simulated images above used to create a Collage. Yes, PixTable shows the wrong values for both FITS image loaded directly with Open Files plugin and also wrong for the buffer in Collage. The highlighted number in the middle of PixValue table does not match the number reported by Value when I mouse around the images.

I just tried several j...fits images in my testdata directory and they are all working fine (image reported in Value matches highlighted value in PixTable). Is the top commit of your PR 27169ce ? Could you drop one of the problem images into the shared Box directory? I'm puzzled by this, because I was having that exact problem, but solved it in commit 06ad686.

@ejeschke
Copy link
Owner Author

Yes, it ran. But I don't see it here.

I see what you mean now. Wonder what the heck is going on with that. I'll try rebasing another one of the PRs and see if the same thing happens.

@ejeschke
Copy link
Owner Author

I just rebased the reproject-plugin PR #813. It looks like it is only running the readthedocs build check. I'll check on the settings in github, but I don't think that problem is related to this PR. Somehow the regular Travis CI build is not connecting to github any more.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Maybe your Travis CI was still using GitHub Services? https://developer.github.com/v3/guides/replacing-github-services/

@ejeschke
Copy link
Owner Author

I suppose so. I don't remember touching that for a long time.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Image link: https://stsci.box.com/s/3k009ufudki2c5lcug1qjwnscbpqw2wm

What I see (PixTable says 86.28 but Value says 8352.729). The cursor was not captured but it was centered at the given X and Y in the screenshot right in the middle of the bright thingy.

pixtable_wrong_value

I am using Ginga via SSH, so I am not physically in front of the machine where it is running. Does this matter though?

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

p.s. Yes, I picked up the latest changes from this PR. The field-info error is gone. Thanks!

@ejeschke
Copy link
Owner Author

Apparently, ginga should have been auto-migrated on February 1, 2019. I guess that didn't work...

@ejeschke
Copy link
Owner Author

The field-info error is gone.

Not sure why you were getting a None channel in that callback. I couldn't reproduce it. But good to have checks in the callbacks anyway...

@ejeschke
Copy link
Owner Author

ejeschke commented Apr 24, 2020

Screenshot from 2020-04-23 15-52-46

@pllim, I am seeing matching values. Your PixTable cutout looks all wrong. Any chance you can try this locally instead of display-back (although that shouldn't matter...).

@ejeschke
Copy link
Owner Author

Maybe your Travis CI was still using GitHub Services?

github shows a web hook registered for Travis. History of events shows the hooks being called. Travis builds show that the hooks are triggering builds.

@pllim, is astropy using the "Travis CI by travis-pro" github "app"? Seems like that may be necessary for the result to show up in the "checks" area of the PR pages...

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Re: PixValue -- It is already broken on master, so I opened a new issue at #836. Due to COVID-19, I cannot check that same machine physically. PixValue works fine on local machine but it is a different OS.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Re: segfault -- Also on master, see #837

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

I think the blockers are all not related to this PR. Would still be nice to see Travis CI actually reporting back before merge but also not a blocker.

@ejeschke
Copy link
Owner Author

Would still be nice to see Travis CI actually reporting back before merge but also not a blocker.

You can see the Travis result passing here. Did astropy do anything when migrating from the service to the web hook?

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

I can't speak for astropy but I switched my projects to GitHub Apps.

Settings -> Integrations -> Installed GitHub Apps

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

I also see it under Settings -> Webhook -> https://notify.travis-ci.org/ (...)

@ejeschke
Copy link
Owner Author

I also see it under Settings -> Webhook -> https://notify.travis-ci.org/ (...)

So that is that is confusing, because this is the exact webhook that is shown in Ginga repo settings. I'm wondering if I also need to install the github app in order to get the result to show up in the checks tab.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Well, doesn't hurt to try. The webhook is obviously sending jobs to Travis CI, just somehow not reporting back.

@ejeschke
Copy link
Owner Author

Thanks for filing the new issues, @pllim. In the interest of working through the backlog of PRs, I'm going to merge this one. I will add some more documentation to clarify usage of Collage before we make the next release.

@pllim
Copy link
Collaborator

pllim commented Apr 24, 2020

Sounds good. Thanks!

@ejeschke ejeschke merged commit cdd1d77 into ejeschke:master Apr 24, 2020
@ejeschke ejeschke deleted the viewer-image-proxy branch April 24, 2020 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants