pmelchior / scarlet2

Scarlet, all new and shiny
MIT License
14 stars 3 forks source link

Consistent coordinates #66

Closed b-remy closed 2 months ago

b-remy commented 3 months ago

This PR solves #51 . The code will be able to accept either pixel coordinates and quantities as well as sky coordianates and astropy quantities. Here are some details of the implementation:

  1. frame.get_pixel now expects SkyCoord objects (or list of) and returns pixel coordinates as arrays and frame.get_sky_coord expects pixel coordiantes as arrays and returns SkyCoord (or list of), to remove any ambiguity. If it's an array, then it is pixel coordinates, if it is SkyCoord, then it is sky coordinates.
  2. Anytime a SkyCoord of u.Quantity object is used in __init__, it looks for the scene and convert it to pixels
  3. Same thing for Parameter, but in addition: a. I had to move the content of set_constraint() from the Parameter.__init__ to Parameter.__iadd__because we need to convert the constraints parameters to pixels before checking the constraints, which expects arrays and floating arguments. b. numpyro priors require a special treatement because we not only need to update the distribution parameters (e.g. loc and scale) but also the batch and event shape. For instance
    numpyro.dist.Normal(loc=SkyCoord(ra=0, dec=0, unit='deg'), scale=1*u.arcsec)

    would set batch_shape=0 and event_shape=0 and converting loc and scale to pixels does not automatically update the batch_shape, while

    numpyro.dist.Normal(loc=[0., 0.], scale=1)

    would set batch_shape=2 and event_shape=0

Here is a notebook which uses SkyCoord and u.Quantity for all fieldname in ['node', 'constraint', 'prior', 'stepsize']

b-remy commented 2 months ago

Simplified frame.get_sky_coord as suggested and changed frame.get_pixel accordingly. Also updated the conversion now performed in arcsec. The tests in the notebook still pass!

b-remy commented 2 months ago

Right, thank you @pmelchior !