WIP: Overview Documentation for PSF Fitting Workflow

[Onoddil]

This pull request is set up for the discussion of the overall PSF fitting process documentation, the fitting workflow and how each block interacts internally, and should not be merged until after the discussion is finished. This documentation complements that in #721 where the documentation for those blocks implemented, with solid APIs unlikely to change in future releases, and the newer APIs available in PRs #753, #755 and #756 for which details may be subject to change.

The main issues for discussion in this document are mostly limited to formatting and clarity of wording, as discussion of individual blocks should be directed to their respective PRs. The issues are summarised below:

• [ ] High Level Overview
  • [ ] Name
  • [ ] Wording
  • [ ] Formatting
  • [ ] Workflow graphic representation

One issue that should be discussed is the final graphical representation of the workflow. The current graph is simply a reproduction of the block diagram, but this PR is meant to supersede that (and closes #761), and so confirmation of the workflow represented here is merited.

Please provide any feedback on the description of the overall PSF fitting process you may have, such that the implementation meets all of the requirements of all users and is as clear as possible going forward. Attached is the final output of the graph for viewing, as currently graphviz is not viewable within this PR.

[marthaboyer]

@Onoddil This all looks fantastic to me! I agree with Eriks suggestions & don't really have much else to add.

[Onoddil]

Thanks @eteq and @marthaboyer for proofs! I've made most of these changes without comment as they're good suggestions, but I've replied to a few inline comments for further question.

With regards to the larger comment of how BasicPSFPhotometry interacts with IterativelySubtractedPSFPhotometry, I'm not sure how this should be tackled. I don't like that we focus so much on a single iterative fitting algorithm (as of writing it's the only one available, but see #732 for a new one...). The concept is that a "Basic PSF fit" is (see the second paragraph of this document for the basic flow, where I thought I discussed this but perhaps not obviously enough): image -> finder (optionally) -> grouping -> fitting -> sources. An "Iterative PSF fit" does those things, then goes to culler and ender, potentially interacts with image again (IterativelySubtractedPSFPhotometry, by the name, subtracts sources found in one iteration and re-fits the residuals, e.g.), potentially interacts with guess at sources (#732 stacks all previously detected sources to be re-fit together, instead of fitting sources in residuals, so it swaps image editing for output table editing), then goes back to finder... Thus one could actually view a "Basic PSF fit" as an "Iterative PSF fit" with culler and ender defined to never cull any sources but always end the 'loop'.

However, as I imagine we can't really get rid of BasicPSFPhotometry (as much as I'd like to just define a "Basic PSF fit" as "pick your favourite iterative routine but don't worry, set Culler and Ender to this BasicCullerEnder and it'll not make any difference"), I think we could better factor the codebase to use as much of BasicPSFPhotometry in subsequent subclasses as possible to highlight that it's basically just one iteration of the code. Thus while a BasicFit is image -> finder (optionally) -> grouping -> fitting -> sources, IterativePSFPhotometry is BasicFit -> culler and ender -> (maybe end or) -> mess with image -> BasicFit -> ... -> end -> sources.

As currently written I don't think it's quite so simple but I don't think the refactor to define things as such is too difficult; BasicPSFPhotometry and iterative subclasses do the heavy lifting in do_photometry but if that were changed so BasicPSFPhotometry essentially has def do_photometry(self, ...): return do_single_loop(self, ...) then subclasses could overload do_photometry but still use the super do_single_loop function.

For this documentation (instead of me waxing philosophical about what I'd like to do to the code...): the attempt to explain how IterativePSFPhotometry builds off BasicPSFPhotometry is in paragraph two. I added the box around the basic PSF parts of the block diagram, but it's basically everything except the culler and ender. I also added the part about "messing with the image" as an optional block. I think (given that I've just reproduced someone else's original block diagram) that the "subtract" floating between fitter and culler and ender was IterativelySubtractedPSFPhotometry's "subtract fit sources from image and find more in the residuals", so we should actually probably more generalise that either way, as that isn't true for all algorithms.

Let me know if the improvements to the block diagram help, and then any suggestions for how to better explain that IterativePSFPhotometry is the entirety of BasicPSFPhotometry + Culler and Ender + Tweak Input Image as a single iteration.

[Onoddil]

Hi @marthaboyer and @eteq - as per a conversation I had offline with @eteq I've pushed the changes I made in response to the comments that were straightforward, but have a few inline replies to comments for clarifications, and the more general query in the comment above. I would appreciate it if you could review the changes I already made, have a look at the queries I had to a few of the comments you had previously, and have a think about the larger issue in my reply above this one.