Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-15536

Add level of indirection in defining Visits from Exposures

    Details

    • 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

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            I have split some of this work off onto DM-23445.

            Show
            jbosch Jim Bosch added a comment - I have split some of this work off onto DM-23445 .
            Hide
            jbosch Jim Bosch added a comment -

            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.

            Show
            jbosch Jim Bosch added a comment - 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.
            Hide
            jbosch Jim Bosch added a comment -

            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.

            Show
            jbosch Jim Bosch added a comment - 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.
            Hide
            jbosch Jim Bosch added a comment -

            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.

            Show
            jbosch Jim Bosch added a comment - 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.
            Hide
            jbosch Jim Bosch added a comment -

            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:

            Show
            jbosch Jim Bosch added a comment - 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
            Hide
            tjenness Tim Jenness added a comment -

            Looks okay. I've got a few comments but nothing major.

            Show
            tjenness Tim Jenness added a comment - Looks okay. I've got a few comments but nothing major.
            Hide
            nlust Nate Lust added a comment -

            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.

            Show
            nlust Nate Lust added a comment - 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.

              People

              • Assignee:
                jbosch Jim Bosch
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Tim Jenness
                Watchers:
                Gregory Dubois-Felsmann, Jim Bosch, Michelle Gower, Nate Lust, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel