Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: daf_butler
-
Story Points:8
-
Epic Link:
-
Team:Data Release Production
-
Urgent?:No
Description
In discussions with Robert Lupton, we decided that it would be best for multiple mutually-inconsistent associations of exposures (snaps) into visits should be permitted in the same Gen3 data repository. We cannot assume that we will be able to do that association in advance and not want to revisit it in later reprocessing.
On the database side, I think we'd need to add a field to the visit table indicating which of several named systems the visit belongs to, while ensuring that visit IDs are unique across all systems. We'd select exactly one system via configuration to be used by any butler client, though we'd need to think about when we'd have to be able to refer to other systems instead of relying on that selection (e.g. when interacting with datasets produced on some other system). We'll also need to move the exposure.visit field into a separate join table.
The way information about visits will be transmitted from the LSST hardware is via a "GROUP" header card; to make use of that, we should:
- propagate that through ObservationInfo into the exposure table during raw ingest;
- remove visit definition (and all of the stuff dealing with visit regions) from raw ingest;
- add a new task to be run after ingest that creates visits (with regions, where appropriate) from groups of exposures with the same GROUP value. That would involve generating visit IDs and names from GROUP via some algorithm.
For instruments that want visits to be 1-1 with exposures, we'll just define GROUP accordingly. If we ever want to define a set of visits that isn't strictly derived from GROUP, we can write an alternate version of the task that populates the same tables via some other means.
Attachments
Issue Links
- blocks
-
- Done
- is blocked by
-
- Done
- is duplicated by
-
DM-23843 Incremental ingest of raw images into Gen3 repository
- Invalid
- is triggered by
-
- Implemented
Activity
| Risk Score | 0 |
| Remote Link | This issue links to "Page (Confluence)" [ 17995 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 20471 ] |
| Description |
In discussions with [~rhl], we decided that it would be best for multiple mutually-inconsistent associations of Exposures (snaps) into Visits should be permitted in the same Gen3 data repository. We cannot assume that we will be able to do that association in advance and not want to revisit it in later reprocessing.
This will entail adding a new DataUnit (proposed name is "SnapSystem") that defines a named association of Exposure into Visits, along with a join table that relates Exposure, Visit, and SnapSystem. The visit field will be removed from Exposure itself, and some metadata from Visit will be moved or copied to Exposure (including at least the region). |
In discussions with [~rhl], we decided that it would be best for multiple mutually-inconsistent associations of exposures (snaps) into visits should be permitted in the same Gen3 data repository. We cannot assume that we will be able to do that association in advance and not want to revisit it in later reprocessing.
On the database side, I think we'd need to add a field to the visit table indicating which of several named systems the visit belongs to, while ensuring that visit IDs are unique across all systems. We'd select exactly one system via configuration to be used by any butler client, though we'd need to think about when we'd have to be able to refer to other systems instead of relying on that selection (e.g. when interacting with datasets produced on some other system). We'll also need to move the {{exposure.visit}} field into a separate join table. The way information about visits will be transmitted from the LSST hardware is via a "GROUP" header card; to make use of that, we should: * propagate that through ObservationInfo into the exposure table during raw ingest; * remove visit definition (and all of the stuff dealing with visit regions) from raw ingest; * add a new task to be run after ingest that creates visits (with regions, where appropriate) from groups of exposures with the same GROUP value. That would involve generating visit IDs and names from GROUP via some algorithm. For instruments that want visits to be 1-1 with exposures, we'll just define GROUP accordingly. If we ever want to define a set of visits that isn't strictly derived from GROUP, we can write an alternate version of the task that populates the same tables via some other means. |
| Labels | gen3-middleware | gen2-deprecation-blocker gen3-middleware |
I'm flipping the dependency relationship between this ticket and DM-23445 after thinking about it a bit more: I'll do the data model changes here first, without trying to modify the ingest script.
| Story Points | 4 | 2 |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Labels | gen2-deprecation-blocker gen3-middleware | gen2-deprecation-blocker gen3-middleware gen3-registry-incompatibility |
| Story Points | 2 | 8 |
I think this is ready to start review - I still have to test and probably fix the ci_hsc packages, but I expect those changes to be trivial and probably won't involve earlier packages.
The functionality here includes both what's in this ticket's description and what's in the description for DM-23445. I've already moved all of the story points to this ticket, and will close the other when I close this one (unless there is a different preferred way to capture "I did the work for this ticket on the branch for another one).
Most code is in obs_base, but there was a significant amount in daf_butler, too, because the new dimensions added revealed some gaps in the systems for relating dimensions. There are trivial changes in the Gen3-supported obs_* packages.
| Reviewers | Tim Jenness [ tjenness ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Previous optimism on finishing this today was, well, optimistic, and I'm glad nobody spent a Tucson holiday trying to review this quickly 
First problem was that I discovered that raw and postISRCCD were still defined in terms of visits, rather than exposures, so I had to change that (Christopher Waters, I hope that doesn't cause you any trouble, but I'm confident that exposure is correct for these).
Debugging that led me to discover a bug in QuantumGraph generation that led to some really confusion error messages, so I spent some time fixing that.
After that, I discovered that the QuantumGraph generation code doesn't see the visit-exposure relationships with the way it works now, and hence it generates bad quanta after this change (postISRCCDs for all exposures get passed to the CharacterizeImageTask quantum for each visit). The change Nate Lust is already working on shouldn't suffer from that problem, but I don't think it's feasible to wait for that. I have another idea in mind, and I think I can get it done early next week.
| Status | In Review [ 10004 ] | In Progress [ 3 ] |
Ok, this is actually ready for full review now - finally passing Jenkins, including ci_hsc.
As this is now even bigger than before, I think splitting up the review makes sense:
- Tim Jenness: all but the last 2 daf_butler commits, obs*, ci_hsc*
- Nate Lust: last 2 commits in daf_butler, pipe*, ip_isr
Full suite of PRs is:
- daf_butler: https://github.com/lsst/daf_butler/pull/254
- pipe_base: https://github.com/lsst/pipe_base/pull/126
- obs_base: https://github.com/lsst/obs_base/pull/224
- ip_isr: https://github.com/lsst/ip_isr/pull/133
- pipe_tasks: https://github.com/lsst/pipe_tasks/pull/374
- obs_decam: https://github.com/lsst/obs_decam/pull/139
- obs_lsst: https://github.com/lsst/obs_lsst/pull/197
- obs_subaru: https://github.com/lsst/obs_subaru/pull/267
- ci_hsc_gen2: https://github.com/lsst/ci_hsc_gen2/pull/15
- ci_hsc_gen3: https://github.com/lsst/ci_hsc_gen3/pull/17
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Looks okay. I've got a few comments but nothing major.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
Some comments I mentioned, in addition to clarification discussions we had on implementation. If I missed any commits to review, let me know, but I think that it is all there.
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
| Epic Link |
|
| Urgent? | off |
I have split some of this work off onto
DM-23445.