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

Set doRenorm default to False in AssembleCcdTask

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ip_isr

      Description

      Change the default value of AssembleCcdConfig.doRenorm to False for the reasons given in RFC-157 and to implement that RFC.

        Attachments

          Issue Links

            Activity

            Hide
            krughoff Simon Krughoff added a comment -

            I think we should remove the option.

            I find it confusing.

            Show
            krughoff Simon Krughoff added a comment - I think we should remove the option. I find it confusing.
            Hide
            rowen Russell Owen added a comment -

            Despite several votes to remove the option, we decided to keep it for reasons given in RFC-157

            Show
            rowen Russell Owen added a comment - Despite several votes to remove the option, we decided to keep it for reasons given in RFC-157
            Hide
            rowen Russell Owen added a comment -

            Would you be willing to look at this trivial change? I'd especially like to know if the wording of the documentation for the doRenorm field is adequate. Please suggest any changes you think would clarify it!

            I still have to remove the override from obs_* packages and make sure that lsst_dm_stack_demo gives the same answers as before.

            Show
            rowen Russell Owen added a comment - Would you be willing to look at this trivial change? I'd especially like to know if the wording of the documentation for the doRenorm field is adequate. Please suggest any changes you think would clarify it! I still have to remove the override from obs_* packages and make sure that lsst_dm_stack_demo gives the same answers as before.
            Hide
            rowen Russell Owen added a comment -

            I updated obs_* packages and also one broken test in pipe_tasks. So all changes complete except possibly lsst_dm_stack_demo and seeing if Jenkins runs

            Show
            rowen Russell Owen added a comment - I updated obs_* packages and also one broken test in pipe_tasks. So all changes complete except possibly lsst_dm_stack_demo and seeing if Jenkins runs
            Hide
            price Paul Price added a comment -

            Please check obs_subaru for overrides of this parameter and remove them (I'm pretty sure there's one).

            Some time ago I warned you not to check image parameters too closely in pipe_tasks. This is the second time you've updated the expected values in a week. Do you still think it's a good idea? Your commit message should justify the changes and show that they're reasonable given the change you've made.

            Show
            price Paul Price added a comment - Please check obs_subaru for overrides of this parameter and remove them (I'm pretty sure there's one). Some time ago I warned you not to check image parameters too closely in pipe_tasks. This is the second time you've updated the expected values in a week. Do you still think it's a good idea? Your commit message should justify the changes and show that they're reasonable given the change you've made.
            Hide
            price Paul Price added a comment -

            Oh, and be careful when merging — I think you branched pipe_tasks from another ticket branch.

            Show
            price Paul Price added a comment - Oh, and be careful when merging — I think you branched pipe_tasks from another ticket branch.
            Hide
            rowen Russell Owen added a comment -

            After I initially sent this for review I updated all obs_ packages including obs_subaru (which had the override in two place).

            Regarding testProcessCcd.py: a larger group discussed this and I implemented what was decided. I lean more towards your viewpoint, but don't want to re-open the issue on this particular ticket.

            Thank you for the warning about pipe_tasks!

            Show
            rowen Russell Owen added a comment - After I initially sent this for review I updated all obs_ packages including obs_subaru (which had the override in two place). Regarding testProcessCcd.py: a larger group discussed this and I implemented what was decided. I lean more towards your viewpoint, but don't want to re-open the issue on this particular ticket. Thank you for the warning about pipe_tasks!
            Hide
            rowen Russell Owen added a comment -

            For the record: Jenkins passes. I guess this is not surprising, because `SdssNullIsrTask` does not support renormalization.

            Show
            rowen Russell Owen added a comment - For the record: Jenkins passes. I guess this is not surprising, because `SdssNullIsrTask` does not support renormalization.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                Paul Price
                Watchers:
                Paul Price, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel