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

crval not set before imager.make_noiseless_image is run #17

Closed
lspitler opened this issue Jun 12, 2018 · 10 comments
Closed

crval not set before imager.make_noiseless_image is run #17

lspitler opened this issue Jun 12, 2018 · 10 comments
Assignees

Comments

@lspitler
Copy link
Member

Crafting a PSF using a simulated galaxy, where I hacked together a new instance of a Huntsman imager:

huntsman_galaxy = imager.Imager(optic=huntsman.optic,
                        camera=huntsman.camera,
                        filters=huntsman.filters,
                        psf=galaxy_psf,
                        sky=huntsman.sky)

Solution was to call huntsman_galaxy.get_pixel_coords(centre), which sets self.wcs.wcs.crval and allows imager.make_noiseless_image to run.

@lspitler lspitler changed the title crval not set before imager.make_noiseless_image is run crval not set before imager.make_noiseless_image is run Jun 12, 2018
@lspitler
Copy link
Member Author

@bazkiaei is this still an issue? @AnthonyHorton do you have a suggested fix for us to implement?

@bazkiaei
Copy link

bazkiaei commented Sep 24, 2018

@bazkiaei is this still an issue? @AnthonyHorton do you have a suggested fix for us to implement?

I believe it is solved due to clipping below zero pixel values within "Fix axis 18 WIP" pull request and adding a warning to the code.

@lspitler
Copy link
Member Author

@bazkiaei I just tried running the jupyter notebook code using this branch and it fails on the 5th cell running:

sky_image = huntsman.make_noiseless_image(centre='14h41m -60d54m',
                                         obs_time='2018-04-12T08:00',
                                         filter_name='g')

I get this error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-47b8d37b5389> in <module>()
----> 1 sky_image = huntsman.make_noiseless_image(centre='14h41m -60d54m',
      2                                          obs_time='2018-04-12T08:00',
      3                                          filter_name='g')
      4 
      5 plt.imshow(sky_image)

AttributeError: 'Imager' object has no attribute 'make_noiseless_image'

Do you get the same error?

@bazkiaei
Copy link

@bazkiaei I just tried running the jupyter notebook code using this branch and it fails on the 5th cell running:

sky_image = huntsman.make_noiseless_image(centre='14h41m -60d54m',
                                         obs_time='2018-04-12T08:00',
                                         filter_name='g')

I get this error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-47b8d37b5389> in <module>()
----> 1 sky_image = huntsman.make_noiseless_image(centre='14h41m -60d54m',
      2                                          obs_time='2018-04-12T08:00',
      3                                          filter_name='g')
      4 
      5 plt.imshow(sky_image)

AttributeError: 'Imager' object has no attribute 'make_noiseless_image'

Do you get the same error?

I was getting that error at the first days I was working on Mock Image Generator. I think the reason is that you are on develop branch of gunagala. In that case you should change the branch to image_sime.

@AnthonyHorton
Copy link
Member

@bazkiaei That axes order fix and CRVAL are unrelated. CRVAL is used to set the RA and dec of the 'reference pixel', i.e. it determines where on the sky the imager is pointing. Might be work checking the axis order of CRPIX, though...

@lee It doesn't get set until make_noiseless_image is run because I was assuming that the on sky position of the imager would only become relevant when creating a simulated image, at which point it gets set from the centre argument of the make_noiseless_image call. There's not really any sensible default value, and (at the time, at least) there didn't seem to be any reason for setting it before the make_noiseless_image call. Could add a method for pre-setting CRVAL if there is a need, though?

@bazkiaei
Copy link

@bazkiaei That axes order fix and CRVAL are unrelated. CRVAL is used to set the RA and dec of the 'reference pixel', i.e. it determines where on the sky the imager is pointing. Might be work checking the axis order of CRPIX, though...

@lee It doesn't get set until make_noiseless_image is run because I was assuming that the on sky position of the imager would only become relevant when creating a simulated image, at which point it gets set from the centre argument of the make_noiseless_image call. There's not really any sensible default value, and (at the time, at least) there didn't seem to be any reason for setting it before the make_noiseless_image call. Could add a method for pre-setting CRVAL if there is a need, though?

Oh, I thought this is related to the error that was arising. @AnthonyHorton thank you for informing me.

@lspitler
Copy link
Member Author

@AnthonyHorton the trouble is that make_noiseless_image make a call implying there is a WCS before one has been set if self.sky doesn't have an attribute 'relative_brightness'. The offending line is: pixel_coords = self.wcs.all_world2pix(((coords.ra.degree, coords.dec.degree),), 0) \

I think you just want to set pixel_coords = self.get_pixel_coords(centre) no matter what?

@lspitler
Copy link
Member Author

I'm going to try and fix it and PR it...

@AnthonyHorton
Copy link
Member

@lee Ah, OK. Probably quite a bit of scope for tidying up there. Rather too much setting things as a side effect of doing other things going on, which makes bugs like this more likely.

lspitler added a commit to lspitler/gunagala that referenced this issue Sep 24, 2018
lspitler added a commit that referenced this issue Oct 3, 2018
* use centre to always set CRVAL first thing

See issue: #17

* created get_pixel_coords method for Imager

* remove centre for get_pixel_coords & check for CRVAL

* add :skip: InvalidTransformError to docs/index.rst

* added test of get_pixel_coords

* fix unit=deg problems in SkyCoord

* also unit=deg fix

* another unit=deg fix

* split test and make scope function

* make_noiseless_image unit=deg

* kwargs docstrings & split star_ & centre_ kwargs
@AnthonyHorton
Copy link
Member

Fixed by #25

lspitler pushed a commit that referenced this issue Nov 25, 2019
* Added dark frame generation to Camera, moved other astroimsim code

* Initial implementations of image simulation methods

* Docs build fix

* Remove now redundant astroimsim.py

* Memory use reduction (no longer creates huge temporary arrays)

* Fix off-by-one, array vs Quantity & floating point stopping bugs

* Fix axis 18 (#20)

* Axis order changed

* Camera.py docstring improved

* clipping below zero pixel values

* Correcting test_psf.py

* camera.py docstring improved

* below zero assert added

* below zero and infinit asserts added

* combining pairs

* `@pytest.mark.parameterize` implemented

* Some problems with ...

* something that works for now

* Crval not set as per Issue #17 (#25)

* use centre to always set CRVAL first thing

See issue: #17

* created get_pixel_coords method for Imager

* remove centre for get_pixel_coords & check for CRVAL

* add :skip: InvalidTransformError to docs/index.rst

* added test of get_pixel_coords

* fix unit=deg problems in SkyCoord

* also unit=deg fix

* another unit=deg fix

* split test and make scope function

* make_noiseless_image unit=deg

* kwargs docstrings & split star_ & centre_ kwargs

* @bazkiaei
trying to parametrize as it has been advised

* @bazkiaei
trying to parametrize as it has been advised

* Revert "Merge remote-tracking branch 'upstream/image_sim' into improve-pytest-#26"

This reverts commit 308168f.

* Revert "Merge remote-tracking branch 'upstream/image_sim' into improve-pytest-#26"

This reverts commit 4b4bc04.

* Revert "Revert "Merge remote-tracking branch 'upstream/image_sim' into improve-pytest-#26""

This reverts commit 25a4c04.

* Revert "@bazkiaei"

This reverts commit 94273a2.

* Revert "@bazkiaei"

This reverts commit c811d41.

* Revert "Merge remote-tracking branch 'upstream/image_sim' into improve-pytest-#26"

This reverts commit 308168f.

* Revert "Revert "Merge remote-tracking branch 'upstream/image_sim' into improve-pytest-#26""

This reverts commit 9eb810c.

* Improving psf_pytest.py  #26 (#28)

The `test_psf.py` parametrised and decorated.

* auto pep8

* fix pixellated call

* add notimplemented error for analytical psf

* fix real image generator bug

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

No branches or pull requests

3 participants