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

Copy configs from obs_* packages to ap_pipe

    XMLWordPrintable

Details

    Description

      To comply with RFC-775, we need to copy configs that are currently set in obs_decam, obs_subaru, and obs_lsst (possibly) to ap_pipe. The configs will need to remain set in the obs packages for Gen 2 support.

      Attachments

        Issue Links

          Activity

            krzys Krzysztof Findeisen added a comment - - edited

            See https://confluence.lsstcorp.org/pages/viewpage.action?spaceKey=~jbosch&title=Reorganizing+Top-Level+Packages+and+Pipelines#ReorganizingTopLevelPackagesandPipelines-Guidelinesforwheretomakeconfigurationchanges for guidelines on what the configs in ap_pipe should look like. This may require some Science Pipelines expertise instead of just being a mechanical copy-and-paste.

            krzys Krzysztof Findeisen added a comment - - edited See https://confluence.lsstcorp.org/pages/viewpage.action?spaceKey=~jbosch&title=Reorganizing+Top-Level+Packages+and+Pipelines#ReorganizingTopLevelPackagesandPipelines-Guidelinesforwheretomakeconfigurationchanges for guidelines on what the configs in ap_pipe should look like. This may require some Science Pipelines expertise instead of just being a mechanical copy-and-paste.
            krzys Krzysztof Findeisen added a comment - - edited

            Based on a recent discussion on #dm, it looks like it's better to wait with this until Gen 2 is no longer being used, and to coordinate it with the move to drp_pipe (i.e., DM-30891). So I will revert this to TODO and remove it from the current sprint.

            If there are config changes to obs_* between now and then, it will be easier to redo the config integration from scratch than to try to splice in the changes, so I won't be saving the work done so far.

            I did learn one thing which it would be worth not re-learning, which is that most of the config fields set in coaddBase.py are only applicable to makeWarp, and not assembleCoadd. Specifically, there's no need to override any of the *ApplyExternal* fields for assembleCoadd; they are defined, but not used.

            krzys Krzysztof Findeisen added a comment - - edited Based on a recent discussion on #dm , it looks like it's better to wait with this until Gen 2 is no longer being used, and to coordinate it with the move to drp_pipe (i.e., DM-30891 ). So I will revert this to TODO and remove it from the current sprint. If there are config changes to obs_* between now and then, it will be easier to redo the config integration from scratch than to try to splice in the changes, so I won't be saving the work done so far. I did learn one thing which it would be worth not re-learning, which is that most of the config fields set in coaddBase.py are only applicable to makeWarp , and not assembleCoadd . Specifically, there's no need to override any of the *ApplyExternal* fields for assembleCoadd ; they are defined, but not used.

            Since this is a blocker for DM-32838, but we can't yet remove the obs package configs (latest word from Middleware is that config removal will be the last step in the Gen 3 migration), I plan to explicitly set every field that's touched by an obs config. That should make us independent unless somebody adds a new field to the obs config. Actually optimizing our configs for AP will be left to a later ticket (possibly DM-32838).

            Parejkoj, let me know if this will work for you.

            krzys Krzysztof Findeisen added a comment - Since this is a blocker for DM-32838 , but we can't yet remove the obs package configs (latest word from Middleware is that config removal will be the last step in the Gen 3 migration), I plan to explicitly set every field that's touched by an obs config. That should make us independent unless somebody adds a new field to the obs config. Actually optimizing our configs for AP will be left to a later ticket (possibly DM-32838 ). Parejkoj , let me know if this will work for you.
            Parejkoj John Parejko added a comment -

            What do you mean by "explicitly set every field that's touched by an obs config"? Setting them in config files in ap_pipe, or in a yaml file in ap_pipe? It sounds like this is basically duplicating the configs between the obs packages and ap_pipe? I'd be a bit concerned about making that duplication now, and having it be out of sync when the obs_* config removal happens.

            Parejkoj John Parejko added a comment - What do you mean by "explicitly set every field that's touched by an obs config"? Setting them in config files in ap_pipe, or in a yaml file in ap_pipe? It sounds like this is basically duplicating the configs between the obs packages and ap_pipe? I'd be a bit concerned about making that duplication now, and having it be out of sync when the obs_* config removal happens.

            I thought the goal was to have it be out of sync, instead of using configs that are designed for DRP?

            krzys Krzysztof Findeisen added a comment - I thought the goal was to have it be out of sync, instead of using configs that are designed for DRP?

            Ah, now I understand. Yes, I think your approach is a good one. This is the config we want now, and this lets us keep it the same until the gen2 removal is complete. We probably want to do a diff between these configs, and the obs package ones when the gen2 removal happens, just to see what might have changed.

            Parejkoj John Parejko added a comment - Ah, now I understand. Yes, I think your approach is a good one. This is the config we want now, and this lets us keep it the same until the gen2 removal is complete. We probably want to do a diff between these configs, and the obs package ones when the gen2 removal happens, just to see what might have changed.

            Parejkoj, I'm not sure what the best way to review this is. I certainly don't recommend careful inspection of all 1500 lines of code!

            The config files should be exact copies except for the comments and the "drop the obs config" hacks, which are on separate commits. The pipeline changes are, of course, completely new.

            krzys Krzysztof Findeisen added a comment - Parejkoj , I'm not sure what the best way to review this is. I certainly don't recommend careful inspection of all 1500 lines of code! The config files should be exact copies except for the comments and the "drop the obs config" hacks, which are on separate commits. The pipeline changes are, of course, completely new.
            Parejkoj John Parejko added a comment -

            Thanks for this: I responded on the PR, and other than a couple more comments there, I think this is a good start towards separating out the DRP stuff.

            Parejkoj John Parejko added a comment - Thanks for this: I responded on the PR, and other than a couple more comments there, I think this is a good start towards separating out the DRP stuff.

            People

              krzys Krzysztof Findeisen
              sullivan Ian Sullivan
              John Parejko
              Ian Sullivan, John Parejko, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.