Hey Andreas – what is "the pipeline" in this context? Is this just an assumption embedded in drp_stella, or will you be proposing changes to LSST code?

drp_stella only. LSST I believe has the pixel centers at [0,0]

I compared the new version to the old one and made sure that the resulting spectra and RMS of the wavelength solution are identical for both PIXEL_CENTER = 0.0 and 0.5.

Note that I have filed the ticket PIPE2D-162 to go through the PSF code & decide which parts should be kept. I also extended the ticket PIPE2D-129 (move code from `SpectrumTemplates.hpp` to `Spectra.cc`) to `PSFTemplates.hpp`.

I am not price (to everybody's great relief, I'm sure), but my suggestion would be that you move all the "cleanup" work here onto a separate ticket. There are currently 28 separate commits in total, of which only about 7 or 8 seem relevant to the work actually being ticketed. Cleaning up formatting etc is nice, but a mountain of changes like this is much harder to review than a small, focused change.

Here is an image showing the FiberTrace centers and limits:

And here an image showing a PSF extracted from multiple dithered PSF representations in one FiberTrace:

Currently the integration test in tests/testDRP.py still fails on modern Mac OS X due to SIP. This is getting fixed in PIPE2D-53 which is currently in review. I can wait for PIPE2D-53 to get merged into master before I resubmit this ticket for review, or I could resubmit it now...?

Does this ticket now pass all its tests?

Also, looking at the changesets I wasn't sure if the fiber trace code still uses (0.0, 0.0) as the bottom left of a pixel internally. Does it? If so, please file a ticket to fix that.

I strongly believe that nothing uses (0.0, 0.0) as the bottom left of a pixel anymore, not even internally. All tests are passing.

rhl Can you please send me the lines you wanted me look at?

My concern was the routine ccdToFiberTraceCoordinates but now that I look more carefully I see that it's not converting between CCD and internal FiberTrace coordinates so that's OK.

I don't understand why you need this function, as it looks to me like something that would have been naturally handled via a definition of dataXY::operator-, but that's out of scope. Actually, it only seems to be used in the PSF code which is slated for removal anyway so we definitely don't want to fix it.

Good to merge then?

Yes. I thought that was implicit from the comment, but apparently not.

merged into master