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

Remove gen2 support from obs_cfht

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      obs_cfht is not used by any remaining gen2 validation code. We can therefore remove the gen2 mapper code from the package.

      One remaining issue is that obs_cfht itself is not fully compatible with gen3.

      Consider: DM-26681 (obs_cfht_data) and DM-21865

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I've made a quick pull request that removes gen2 and does not change the test coverage. Jenkins passes fine.

            Show
            tjenness Tim Jenness added a comment - I've made a quick pull request that removes gen2 and does not change the test coverage. Jenkins passes fine.
            Hide
            tjenness Tim Jenness added a comment -

            Eli Rykoff for obs_subaru gen2 removal how did you handle the gen2 tests that are just a butler.get of a calibration? Is the idea that we remove the gen2 stuff from testdata_cfht and then do a test that creates a repo, ingests the raws and calibrations and then sees that they can be retrieved?

            Christopher Waters do we know anything about the CFHT-specific IsrTask? It doesn't seem to be tested and is not listed in a pipeline YAML file. Is the config/processCcd.py where all the magic happens? I assume that's a gen2-specific config that is now irrelevant. For gen3 do we have to define a pipeline that explicitly lists the CFHT IsrTask? I don't know if we test that anywhere even in gen2.

            Show
            tjenness Tim Jenness added a comment - Eli Rykoff for obs_subaru gen2 removal how did you handle the gen2 tests that are just a butler.get of a calibration? Is the idea that we remove the gen2 stuff from testdata_cfht and then do a test that creates a repo, ingests the raws and calibrations and then sees that they can be retrieved? Christopher Waters do we know anything about the CFHT-specific IsrTask? It doesn't seem to be tested and is not listed in a pipeline YAML file. Is the config/processCcd.py where all the magic happens? I assume that's a gen2-specific config that is now irrelevant. For gen3 do we have to define a pipeline that explicitly lists the CFHT IsrTask? I don't know if we test that anywhere even in gen2.
            Hide
            erykoff Eli Rykoff added a comment -

            Re: the gen2 get tests for obs_subaru, I simply deleted them. I assumed that the gen2 code-paths there were covered by ci_hsc_gen3.

            Note that we have similar questions for obs_decam, we filed DM-34862 to convert testdata_decam to gen3 which will allow the migration of these unit tests in DM-34863. I think we need to do the same for testdata_cfht.

            Show
            erykoff Eli Rykoff added a comment - Re: the gen2 get tests for obs_subaru, I simply deleted them. I assumed that the gen2 code-paths there were covered by ci_hsc_gen3. Note that we have similar questions for obs_decam, we filed DM-34862 to convert testdata_decam to gen3 which will allow the migration of these unit tests in DM-34863 . I think we need to do the same for testdata_cfht .
            Hide
            tjenness Tim Jenness added a comment -

            One of the problems is that obs_cfht is not really usable for gen3 because no-one cares enough about it to sort out the fundamentals (see DM-21865 and DM-26681).

            Show
            tjenness Tim Jenness added a comment - One of the problems is that obs_cfht is not really usable for gen3 because no-one cares enough about it to sort out the fundamentals (see DM-21865 and DM-26681 ).
            Hide
            czw Christopher Waters added a comment -

            I think that the existing CfhtIsrTask will work in gen3 as is, as it seems to only be checking gain/read noise values.  I assume that the gain/read noise variations in the data make it important to do that to get useful variance planes, and so it's probably worth retaining.  If you make a ticket for unit test coverage of it, I'm happy to do it.

            Show
            czw Christopher Waters added a comment - I think that the existing CfhtIsrTask will work in gen3 as is, as it seems to only be checking gain/read noise values.  I assume that the gain/read noise variations in the data make it important to do that to get useful variance planes, and so it's probably worth retaining.  If you make a ticket for unit test coverage of it, I'm happy to do it.
            Hide
            tjenness Tim Jenness added a comment -

            As agreed, I've ripped out all the gen2 and have not attempted to deal with porting over the gen2 calibration tests. The raw ingest tests check that gen3 is working.

            Jenkins passes. No rush to review.

            Show
            tjenness Tim Jenness added a comment - As agreed, I've ripped out all the gen2 and have not attempted to deal with porting over the gen2 calibration tests. The raw ingest tests check that gen3 is working. Jenkins passes. No rush to review.
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for doing this cleanup. See my notes: unfortunately, obs_cfht was not included in the DM-30891 work, so that block in the ups table is still necessary.

            Show
            Parejkoj John Parejko added a comment - Thanks for doing this cleanup. See my notes: unfortunately, obs_cfht was not included in the DM-30891 work, so that block in the ups table is still necessary.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              John Parejko
              Watchers:
              Christopher Waters, Dominique Boutigny, Eli Rykoff, John Parejko, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.