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

DM-12798: Add source selection by signal-to-noise ratio #95

Merged
merged 5 commits into from Dec 8, 2017

Conversation

PaulPrice
Copy link
Contributor

No description provided.

The various source selection modules have had common code to apply
minimum and maximum limits. Factor this common code into a base class.


class SignalToNoiseLimit(BaseLimit):
"""Select sources using a magnitude limit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update description


def apply(self, catalog):
"""Apply the magnitude limit to a catalog

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

----------
catalog : `lsst.afw.table.SourceCatalog`
Source to which to apply limits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wording: it’s a catalog of sources to which the limit will be/is/gets applied

Returns
-------
selected : `numpy.ndarray`
Boolean array indicating whether each source is selected
Copy link
Contributor

@laurenam laurenam Dec 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “each” doesn’t seem right here. Replace with “a given” or “...indicating for each source whether it is selected”?

Also note that sources for which the general flag for the fluxField is set will be excluded? Also, since you allow for the error flux field to be different, should you select against its flux flag as well (if different from fluxField...different flux fields failure modes may differ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flags can be handled by the RequireFlags selection, so no need to include them here.

maximum = pexConfig.Field(dtype=float, optional=True, doc="Select objects with value less than this")

def apply(self, values):
"""Apply the limit to an array of values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

limit(s)


def setDefaults(self):
pexConfig.Config.setDefaults(self)
self.flags.bad = ["base_PixelFlags_flag_edge", "base_PixelFlags_flag_interpolated",
"base_PixelFlags_flag_saturated"]
self.signalToNoise.fluxField = "base_PsfFlux_flux"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to default the errField (since having them associated is not assumed?)

high.set("other_fluxSigma", 0.001)
self.config.doSignalToNoise = True
self.config.signalToNoise.fluxField = "other_flux"
self.config.signalToNoise.errField = "other_fluxSigma"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @parejkoj is trying to homogenize the use of err vs. Sigma in the stack (RFC-333).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, none of that has been implemented yet, so fluxSigma will remain.

Bit better wording, with thanks to Lauren MacArthur.
Masayuki Tanaka (NAOJ) suggested that a signal-to-noise ratio limit would
be a useful addition.
If you've got only a magnitude (not flux), you can't use the
SignalToNoiseLimit without some modification. However, for
magnitudes the error is the inverse of the signal-to-noise
ratio, so we can just put a limit on that.
File generated by pytest.
@PaulPrice PaulPrice merged commit b111fa0 into master Dec 8, 2017
@ktlim ktlim deleted the tickets/DM-12798 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants