Skip to content

Added 'Filters Overview' page to the docs #80

Merged
rknop merged 22 commits intoLSSTDESC:mainfrom
sidratresearch:sidrat-staging
Apr 16, 2026
Merged

Added 'Filters Overview' page to the docs #80
rknop merged 22 commits intoLSSTDESC:mainfrom
sidratresearch:sidrat-staging

Conversation

@jscora
Copy link
Copy Markdown
Contributor

@jscora jscora commented Apr 13, 2026

  • created a 'filters overview' page that has some documentation on what filters are, and how to create a new Fink filter
  • some python formatting and correcting typos

Copy link
Copy Markdown
Contributor

@rknop rknop left a comment

Choose a reason for hiding this comment

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

Couple of suggestions.

Comment thread docs/filters.rst Outdated
* apFluxErr (estimated flux uncertainty in nJy)
* visit (id of the visit where the source was measured)
* ra (Right ascension of the center of this source)
* dec (Delination coordinate of the center of the source)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I might also add here that we need the prvDiaSources and prvDiaForcedSources fields from the LSST alerts. (Reason: in the absence of a PPDB, the only way we can get any of this information is from alerts. Forced sources never generate alerts, so we depend on the prvDiaForcedSources field. It's possible that some earlier sources for the same object did not pass the filters, so we may not have received them; thus, we need the prvDiaSources as well, even though that does mean we end up ingesting a huge amount of redundant data.)

The necessary fields in the prv arrays are from the diasource and diaforcedsource FASTDB tables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure exactly what you mean here -- I don't see a prvDiaSources or prvDiaForcedSources field in the LSST alert schema. Though maybe I'm looking in the wrong place -- I've been looking here.

Do you mean there's a whole separate section of the schema for previous sources? And that you want both that list of sources and the parameters for each of those sources from the diaforcedsource and diasource FASTDB tables?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cf: comment in Slack

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the flux from the prvDiaForcedSources array, do you want the psfFlux, scienceFlux, or both?

Comment thread docs/filters.rst Outdated
Comment thread docs/filters.rst
* ra (Right ascension of the center of this source)
* dec (Delination coordinate of the center of the source)


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would also add that ideally we need the diaObject entry in the alert schema as well=. There, I think the only fields we really use are ra, dec, raErr, decErr, and ra_dec_cov, and we can live without the last three.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By this do you mean that we want at a minimum both values from the DiaSource and DiaObject schemas? So this would be two lists, essentially, with the existing list being parameters from the DiaSource schema?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah -- the diaObject positions are fraught, and I'm not fully sure how they're even defined. The diaSource positions, presumably, are the position where the source was detected on the difference image. Ideally the diaObject position is some kind of weighted average of all the diaSource positions for sources that are part of the object. However, when they send the first alert, they only have one source. What I don't know is if they ever update the diaObject position.

I want the diaObject position so that if we don't get postions for sources, we have something to use. But, if we can also get the diaSource positions, we can do our own weighted averages.

Copy link
Copy Markdown
Contributor

@rknop rknop left a comment

Choose a reason for hiding this comment

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

This looks good. I have one small suggestion, three columns in diaObject are only semi-required, and we should probably indicate that.

Let me know when this is happy, and I'll merge it. (I'm going to ignore the failed tests, because that's something I have to clean up (it has to do with the docker images necessary for the tests being available in the github container repo), and because none of this PR changes any code.)

Comment thread docs/filters.rst Outdated
+-----------------+----------------------------------------------------------+
| ``dec`` | Declination coordinate of the center of the source (deg) |
+-----------------+----------------------------------------------------------+
| ``raErr`` | Uncertainty of ra |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps put something that indicates we can live without this for the uncertainties (raErr, decErr, ra_dec_cov). It's good to have, but if we don't, it's not as big a deal. (The postgres columns are nullable.)

Copy link
Copy Markdown
Contributor Author

@jscora jscora Apr 16, 2026

Choose a reason for hiding this comment

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

I removed the rows that are optional -- since I say it's a list of required parameters, I figured it makes the most sense to only have the ones that are actually required

taiwithers and others added 4 commits April 16, 2026 10:17
Also removed redundant link to generic tutorials, to leave specifically
the one for Pub/Sub

Re-ordered links to put the likely less useful broker documentation at
the bottom
@rknop rknop merged commit af3ead1 into LSSTDESC:main Apr 16, 2026
1 of 2 checks passed
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.

3 participants