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

Image pipeline issue with sky image size #925

Closed
cdeil opened this issue Feb 28, 2017 · 5 comments
Closed

Image pipeline issue with sky image size #925

cdeil opened this issue Feb 28, 2017 · 5 comments
Assignees
Labels
Milestone

Comments

@cdeil
Copy link
Contributor

cdeil commented Feb 28, 2017

@leajouvin - I changed the sky image tutorial notebook a little, the current version is here:
https://github.com/gammapy/gammapy-extra/blob/61765830218610d00a11fd413bf4a139bc506894/notebooks/image_pipe.ipynb

It works as before.

But if I change nxpix=300, nypix=300 to nxpix=500, nypix=500, then the background estimate fails completely (all NaN I think, white output). There's also the weird trick to put a maximum FOV offset cut of 2.49, ideally evaluating for larger offsets would work by default (cc @joleroi) and fill zeros or extrapolate, not fail completely.

@leajouvin - Can you reproduce this behaviour? Any idea what the issue is here? Probably having one pixel with offset > offset_max in the model leads to this behaviour? Is that it, or is there some other issue?

@cdeil cdeil added the bug label Feb 28, 2017
@cdeil cdeil added this to the 0.6 milestone Feb 28, 2017
@JouvinLea
Copy link

@cdeil
I get zero values outside the range!
My version for numpy is 1.12.0, for scioy 0.16.1 and for gammapy gammapy.version.version

@JouvinLea
Copy link

@cdeil
There is still a bug to fix for the tests for image_pipe and cube_pipe class concerning nan values...
https://travis-ci.org/gammapy/gammapy/jobs/208692980

@cdeil cdeil modified the milestones: 0.7, 0.6 Apr 28, 2017
@cdeil cdeil modified the milestones: 0.7, 0.8 Nov 5, 2017
@cdeil
Copy link
Contributor Author

cdeil commented Feb 2, 2018

@registerrier @JouvinLea - This tutorial notebook is currently broken:

The error is this:

<ipython-input-2-4e3ff94d0c17> in <module>()
      6 from gammapy.data import DataStore
      7 from gammapy.image import SkyImage
----> 8 from gammapy.scripts import StackedObsImageMaker
      9 from gammapy.utils.energy import Energy

ImportError: cannot import name 'StackedObsImageMaker'

See https://gist.github.com/cdeil/222ce8f3cc81223721173ce918b2b4e9#file-gistfile1-txt-L33


That tutorial is very similar to what we show here:
http://docs.gammapy.org/dev/notebooks/cta_data_analysis.html#Make-sky-images

The plan is that both the StackedImageObsMaker and the IACTImageEstimator will be replaced with new code #1277 and with the HESS test data release in a months we will have a better example dataset.

I propose that we now simply remove the image_pipe tutorial notebook, and in a month write a new one.

The alternative is that @leajouvin or @registerrier you update and fix that notebook now, if you think it's useful to keep.

What do you think?

@JouvinLea
Copy link

@cdeil @registerrier
I just tried with the lastest version of gammapy and it worked fine for me....

@cdeil
Copy link
Contributor Author

cdeil commented May 7, 2018

I removed the image_pipe notebook :

gammapy/gammapy-extra@c6303bd

and also the old cube analysis notebooks:

gammapy/gammapy-extra@2db123d

This is part of the effort to rewrite map-based analysis (both 2D and 3D) in Gammapy and to remove SkyImage / SkyCube and the old image and cube makers and Sherpa 3D fitting code.

gammapy.maps is pretty well-developed, and development of new analysis code on top of it and even tutorial documentation has started:
http://docs.gammapy.org/dev/notebooks/analysis_3d.html
http://docs.gammapy.org/dev/notebooks/simulate_3d.html

It's not very far along, but still, @registerrier and I discussed on Friday, and agreed that keeping tutorials for both old and new in parallel is confusing for users and hard to maintain for us and make progress on the new quickly, so we start by removing high-level tutorials for the old first to stop advertising it to users. We'll do the v0.8 release this week and after that we can remove some more things.

@JouvinLea and @adonath - Thank you very much for all your work on Gammapy and developing SkyImage / SkyCube and the Sherpa-based analysis, and for agreeing to the plan to rewrite. We learned a lot from this, it was very valuable!

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

No branches or pull requests

3 participants