Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

PropagateVisitFlags doesn't work with other pipeline components

Description

PropagateVisitFlags, which was recently ported over from HSC on DM-4878, doesn't work due to some inconsistencies with earlier packages/tasks:

  • The default fields to transfer have new names: "calib_psfCandidate" and "calib_psfUsed"

  • We're not currently transferring these fields from icSrc to src, so those fields aren't present in src anyway. I propose we just match against icSrc for now, since it has all of the fields we're concerned with.

  • It makes a call to afw.table.ExposureCatalog.subsetContaining(Point, Wcs, bool), which apparently exists in C++ but not in Python; I'll look into seeing which HSC commits may have been missed in that port.

Confluence content

mentioned on

Checklist

Lucidchart Diagrams

Issue Matrix

hide

Activity

Show:
Jim Bosch
February 10, 2016 at 10:27 AM

Merged to master.

Commit messages shortened (I hate the fact that my favorite gui uses a variable-width font for commit messages, so I can't tell how long they are sometimes!).

I added some checks to ci_hsc to make sure the flags fields are actually appearing in the output catalogs, but my plan to test that PSF stars were classified appropriately failed due to https://rubinobs.atlassian.net/browse/DM-5109#icft=DM-5109, so I'll defer adding that test to that new issue.

Lauren MacArthur
February 8, 2016 at 2:49 PM

Sounds good. Feel free to merge when you feel comfortable that the current numbers seem at least reasonable.

Jim Bosch
February 8, 2016 at 2:46 PM

I'll look into adding something into the ci_hsc checks to see that the flags being set are reasonable. I'm thinking of checking that we have a nonzero number of sources with the new flags, and that at least most of them are classified as stars on the coadd.

Lauren MacArthur
February 8, 2016 at 2:30 PM

Fair enough, but is there some one-off way to ensure the current behavior is correct? I'm weary of including flags whose behavior has not been demonstrated to function "as advertised". That said, I believe the current master is in a broken state, so I also don't want to delay merging this fix. Let me know your thoughts.

Jim Bosch
February 8, 2016 at 9:01 AM

This will be very hard to test with the mock coadd stuff (there isn't currently any visit processing done there at all). I do think the right place to test this is something like ci_hsc, and that testing is effectively in place because it won't run all the way through without all three of these changes. If any of the other integration test suites based on other cameras extend to building and processing coadds, it may be useful to make sure this is effectively tested there as well.

Done
Pinned fields
Click on the next to a field label to start pinning.

Details

Assignee

Reporter

Reviewers

Lauren MacArthur

Story Points

RubinTeam

Components

Sprint

None

Parent

Checklist

Created February 5, 2016 at 1:32 PM
Updated June 6, 2016 at 10:09 AM
Resolved February 10, 2016 at 10:29 AM