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

Make BF kernel measurement code fully stack compliant

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      70
    • Epic Link:
    • Team:
      Data Release Production

      Description

      This is mainly refactoring as currently lots of code is duplicated.
          * Add full camera agnosticism, will need to run ISR tasks from the obs_package, from outside ISR
              * Probably define this in each obs_package as a config option as to which ISR task to target (default or own)
          * Possibly also speed-up slow bits if things are being done too inefficiently

        Attachments

          Issue Links

          has to be done after

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-13292 Stackify BF kernel measurement code

          • Done
          has to be done before

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-13294 Add debug functionality to BF kernel measurement code

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-15756 biasCorr calculation code results disagrees when using main task code

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-15579 Suggested improvements to makeBrighterFatterKernel.py

          • Won't Fix
          is duplicated by

          Bug - A problem which impairs or prevents the functions of the product. DM-14396 IsrTask mis-handles getting the brighter-fatter kernel

          • Invalid

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-10358 confusion about the brighter fatter kernel file

          • Invalid

          Improvement - An improvement or enhancement to an existing feature or task. DM-14737 Update obs_comCam to work with new isr distortion models

          • Invalid
          is triggering

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-15509 Fix bitrot due to premature merge of DM-13293 in obs_lsstCam

          • Done
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-14190 Port the BF simulation code so that the biasCorr parameter can be measured

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-15648 Specify which BF kernel to use with configuration file

          • Invalid

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-5082 Implement code to generate kernel for brighter fatter correction.

          • Invalid

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            rowen Russell Owen added a comment - - edited

            My first comment is that I am unhappy using "bf" as an abbreviation for "brighter-fatter" in so many really visible places. Our standards are to spell things out unless they are well established norms, as per this entry in the style guide. I guess the question is whether "bf" is now sufficiently well established to qualify.

            If you do change this I suggest changing "bf to "brighterFatter" in the dataset names (which neatly eliminates the collision with the name used by obs_subaru for the brighter-fatter kernel) and the task name BFTask -> BrighterFatterTask and the task _DefaultName to "brighterFatter".

            I also think the name BFTask is too vague because it sounds like a task that applies compensation to data for the brighter-fatter effect, but in this case it appears to compute a kernel that can be used by other code to apply such compensation. Consider a name like MeasureBrighterFatterTask.

            (Note: if nothing else, you should rename BFTask to BfTask – ugh – as per this standard, hence SkyWcs, LsstSimMapper, etc.).

            Show
            rowen Russell Owen added a comment - - edited My first comment is that I am unhappy using "bf" as an abbreviation for "brighter-fatter" in so many really visible places. Our standards are to spell things out unless they are well established norms, as per this entry in the style guide . I guess the question is whether "bf" is now sufficiently well established to qualify. If you do change this I suggest changing "bf to "brighterFatter" in the dataset names (which neatly eliminates the collision with the name used by obs_subaru for the brighter-fatter kernel) and the task name BFTask -> BrighterFatterTask and the task _DefaultName to "brighterFatter". I also think the name BFTask is too vague because it sounds like a task that applies compensation to data for the brighter-fatter effect, but in this case it appears to compute a kernel that can be used by other code to apply such compensation. Consider a name like MeasureBrighterFatterTask . (Note: if nothing else, you should rename BFTask to BfTask – ugh – as per this standard , hence SkyWcs , LsstSimMapper , etc.).
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Looks good. I am very sorry that I lost track of this. Thanks for all your updates to this complex code.

            Show
            rowen Russell Owen added a comment - Looks good. I am very sorry that I lost track of this. Thanks for all your updates to this complex code.
            Hide
            Permalink
            mfisherlevine Merlin Fisher-Levine added a comment -

            No problem, thanks for the massive review and excellent suggestions!

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - No problem, thanks for the massive review and excellent suggestions!
            Hide
            Permalink
            mfisherlevine Merlin Fisher-Levine added a comment -

            Merged.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Merged.
            Hide
            Permalink
            yusra Yusra AlSayyad added a comment -

            Merlin Fisher-Levine, will you add a realistic number of story points to this ticket and double check the epic?

            Show
            yusra Yusra AlSayyad added a comment - Merlin Fisher-Levine , will you add a realistic number of story points to this ticket and double check the epic?

              People

              Assignee:
              mfisherlevine Merlin Fisher-Levine
              Reporter:
              mfisherlevine Merlin Fisher-Levine
              Reviewers:
              Russell Owen
              Watchers:
              Christopher Waters, Merlin Fisher-Levine, Russell Owen, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.