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-18318: Create initial subset of timeseries features for DIAObject #43

Merged
merged 7 commits into from Mar 19, 2019

Conversation

morriscb
Copy link
Contributor

No description provided.

Add totFlux DiaObject columns.

Add totFlux columns to afw schema.

Fix TotFluxSigma naming.

Change totFlux column order.

Fix unittests.

Fix up linear fitting.
# The first commit's message is:

Hook up StentsonJ calculation.

# This is the 2nd commit message:

Debug StetsonJ calculation.
Fix case mismatch in column names.
Fix flux masking.
Copy link
Contributor

@ebellm ebellm left a comment

Choose a reason for hiding this comment

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

A few small changes

type: FLOAT
nullable: true
description: Linear best fit slope of the u band fluxes.
unit: nJy/MJD
Copy link
Contributor

Choose a reason for hiding this comment

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

should be nJy/day (MJD is a time system, not a time unit). Same for the other slopes here.

@@ -104,6 +104,7 @@ def make_dia_object_schema(filter_names=None):
doc='Standard error of uPSFluxMean.')
schema.addField('uPSFluxSigma', type='F',
doc='Standard deviation of the distribution of uPSFlux.')

Copy link
Contributor

Choose a reason for hiding this comment

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

extraneous whitespace?

@@ -120,6 +121,7 @@ def make_dia_object_schema(filter_names=None):
doc='Standard error of gPSFluxMean.')
schema.addField('gPSFluxSigma', type='F',
doc='Standard deviation of the distribution of gPSFlux.')

Copy link
Contributor

Choose a reason for hiding this comment

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

extraneous whitespace?

return ans


def _stentson_J(fluxes, errors):
Copy link
Contributor

Choose a reason for hiding this comment

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

"stentson" -> "stetson" (someone's name); fix throughout

totFluxErrors = totFluxErrors[noNanMask]

if len(totFluxes) == 1:
dia_object_record['%sFPFluxMean' % filter_name] = totFluxes
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call these totFluxMean and so on so the link to the DIASource quantity is more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I initially had too, however, FPFlux is coming from the current version of the cat schema and the schema in dax_ppdb. I'm trying to reuse those two as much as possible so things don't get even more confused with the column names. I can add totFlux columns to the schema extra files and just leave the FPF names empty but that might be confusing for folks using the current setup. Hopefully once we have a automated way of making yaml schemas from the DPDD soon.

Copy link
Contributor

@ebellm ebellm Mar 19, 2019

Choose a reason for hiding this comment

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

I just think FPFlux should refer to a different thing, which is means/errors on the diffim forced fluxes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so would you prefer that I add them in as specifically totFlux here and in the schema files?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that would be my preference

"""Compute the Stentson mean of the fluxes which down-weights outliers.

Weighted biased on an error weighted difference scaled by a constant
(1/``a``) and raised to the power beta. Higher beta's more harshly penalize
Copy link
Contributor

Choose a reason for hiding this comment

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

"beta's" -> "betas" (or "values of beta")

Returns
-------
stentsonJ : `float`
StentsonJ statistic for the input fluxes and errors.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add a citation note to Stetson 1996

Returns
-------
wmean : `float`
Weighted Stentson mean result.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add a citation note to Stetson 1996

Add TOTFlux explicitly.

Hook up TOTFlux output.

Fix linting.
@morriscb morriscb merged commit 89c5465 into master Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants