Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Data Noise Handling Future PSF Photometry API #755

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Onoddil
Copy link
Contributor

@Onoddil Onoddil commented Oct 25, 2018

This pull request is set up for the discussion of the forward-looking additions to PSF photometry and should not be merged until after the discussion is finished. In this case the issue is the 'data noise' (previously the 'noise_model'; the name of the block has been changed to avoid potential confusion with 'psf_model') aspect of the block diagram. This API documentation complements that in #721 where the documentation for those blocks implemented, with solid APIs unlikely to change in future releases. Please see #766 for an overview of the fitting process and a revised block diagram.

The structure of the blocks input and output parameters is described in detail in the documentation, but the main issues are summarised below:

  • NoiseData
    • Name
    • Formatting
    • Wording
    • Callable function vs more internal routine
    • Overload inputs keywords/function acceptable
    • Handling of conversion from uncertainty to weight keyword in fitter
    • API workflow: requires data (array-like) and returns uncertainty (array-like)
    • Implications for aperture

In particular discussion on

  • whether to revert the name back to 'noise_model', if this is too easily confused with 'psf_model', or if a third name is a better descriptor
  • whethernoise_calc should be a simple callable function or a more internal routine of some kind
  • whether keyword overload (e.g., None allows for the simple effective unit-weighting of all data values and some secondary keyword, as yet unimplemented, to indicate that uncertainty values have been passed in with the data values and to ignore the computation of uncertainty values) is acceptable, and what such keywords should be
  • the particular conversion of "uncertainty" to the weight keyword of fitter functions (i.e., is weight = 1/uncertainty, are uncertainty values standard deviations or variance, etc)
  • the implications of this API block -- primarily for PSF fitting -- on aperture fitting. Current implementation requires the update of aperture_photometry to accept the uncertainty argument (instead of its current error keyword) to correctly handle NDData arrays via @support_nddata. Nominally a relatively minor and cosmetic keyword update, is it acceptable to force an API change on another part of photutils or must uncertainties be handled in a different way in the PSF fitting routines to avoid this conflict?

is appreciated to finalise the API call.

Please provide any feedback on the API for this PSF photometry fitting routine block you may have, such that the implementation meets all of the requirements of all users and is as clear as possible going forward. A simple example for this block, maintaining "do nothing" backwards-compatibility can be found in IterativelySubtractedPSFPhotometry. It should be noted that the code provided above does not include examples of keyword overloading for uncertainty passing, simply accepting the callable function to compute uncertainties based on data values. The changes made to support NDData in aperture_photometry can be seen here. Please also provide simpler formatting or grammatical changes to improve the readability and professional look of the document. The function's primary goal is to allow for the inclusion of realistic uncertainty weights in the fitting routine within the PSF fitting process, allowing for the in-situ calculation of such uncertainties or the passing of pre-determined uncertainties.

Base automatically changed from master to main March 16, 2021 02:46
@larrybradley larrybradley marked this pull request as draft March 17, 2021 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants