Closed pmelchior closed 3 years ago
Did you branch this off of deconv_init
and are trying to merge to master?
I will review this after we merge 227 as the diff is currently huge and largely dominated by 227.
That's what I suggested
A few comments:
ResolutionRenderer
is a straight copy from LowResolutionObservation
Observation.data
I was thinking of spectroscopic data sets, but this is a little premature. But I'd rather make this change now.Renderer
(which can decide if it is even possible to create), but I have removed the deconvolution init because it seems to not help. Since the code is in master, we can bring it back if we want to.
Christmas PR!
This implements the suggestions in #220. In particular, it defines
Observation
as aFrame
with a data payload and alog_likelihood
method. The actual rendering method is provided by subclasses ofRenderer
. This avoids having to change the type of observation. Instead,Observation.match
sets/changes the renderer.As an additional bonus, this branch derives
Renderer
fromModel
, reflecting the fact that the mapping from model space to data space can include optimizable parameters (WCS, PSF, mag zeropoint #212, wavelength calibrations...). So,Renderer
creates the parameterized transformation function and then applies it to the given model. This will serve us well for cases where we'd like to fit for the PSF in a crowded field etc.Finally, this branch builds off
prerender
which is currently under review in #227. I suggest to do that PR first, and then move to this one, otherwise there are too many moving parts.