Do we have an issue for making correctfluxes code available in the new framework? Please make sure it's linked to this one, so this doesn't get forgotten.

I assume the removal of computeSincFlux from meas_algorithms is covered by DM-420, so I've linked that issue. (I'm not sure I understand why deferring this work until that issue is addressed is desirable, but it doesn't seem to be a big deal).

Jim Bosch Shall we add this to DM-1768 for S15?

"correctfluxes" was the old name for the code that applied aperture corrections, so the issue to replace it is epic DM-85. I'd rather remove that code here than comment it out, though (in general, we should never have commented-out code on master; if it's a block we think we might want to reenable, use #if 0, and if we just want to be able to see what the code was, we should use git).

DM-420 is the correct issue for removal of calculate SincApertureFlux. I'm hoping we can get that done in the next sprint, but if we don't, it would land in DM-1768.

I decided to defer the work because it was neither really part of the original ticket, nor did it seem to me wise to do it without some thought and consultation. I believe it would involve either rewriting the internals of the KronAlgorithm, or the routine in ApertureFlux. The problem is an inconsistency in the templating, between the two modules. We are more likely to get it right on a new ticket at the start of the next Sprint. I am happy to do the work then.

I looked at the changes made in DM-982 regarding "correctfluxes". By my (superficial) reading, they look as if they're simply to prevent errors due to the algorithm no longer being available. If that's correct, then it looks to me as if that code can just vanish (and maybe the review of DM-982 will say that); when a replacement algorithm is implemented, if it causes similar errors, they'll naturally be fixed as part of adjusting the tests to handle that new algorithm.

As for the sinc flux, it seems like it would have to be handled when the relevant routine is removed from meas_algorithms, otherwise it would cause obvious breakage. It doesn't seem like we need a separate story to track that.

Given the above, I'm a bit minded to think there's nothing here that really requires a standalone story. Anybody object to my just shutting it down?

The code is there is comments as a convenience for the person who has to
fix it when flux correction is implemented. It is not crucial that it be
in the code, it is just a reminder. If that is frowned upon, I still would
like to havean explicit reminder about this in some ticket, citing that the
original Kron code in meas_extensions had it and that it was not ported.
Since this code is in an extension, it could easily be forgotten.

The only reason to have a ticket for the SincFlux stuff is if you feel like
it should be done independently of the other work. If not, kill it.

On Thu, Feb 5, 2015 at 1:41 PM, John Swinbank (JIRA) <jira-dm@lsst.org>

My belief is that there is nothing left to do on this ticket. John, can you confirm?

I removed the comments as part of an earlier review, and fixed the issue with moving the SincFlux calculation to the new ApertureFlux.cc code as part of DM-420.

As I recall, the comments in point 1 were there to remind us to add aperture corrections to meas_extensions_photometryKron when the relevant functionality was available. I think comments aren't a great way to do that, so I'm glad they're gone. But is it still important that photometryKron be updated when DM-85 is done? If so, either this issue or another needs to be tracking that.

I agree that point 2 is addressed by DM-420.

John, can you move this issue to review complete or defer it, depending on whether you think that there is still work to be done on it? I think the only open issue has to do with aperture corrections.

Hi Perry – my comment above is still outstanding. If changes will need to be made to _photometryKron when DM-85 is done, we need a way of tracking that. It could be this issue, or it could be another issue, but it should be clear which. If those changes don't need to be made, then we can close this issue, but we need a clear statement to that effect.

i agree. So I will just take it out of the review loop and defer it.

Actually, I guess there is no way for me to mark it as deferred, so will just put it is a working state again and you can move it to another sprint.

Since this ticket covers multiple issues, I think it makes sense to close this one down now and file a specific ticket for the aperture corrections to _photometryKron. So I've done that; new ticket is DM-2429.