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

Make BF kernel measurement code fully stack compliant

    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

            Activity

            Hide
            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
            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
            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
            mfisherlevine Merlin Fisher-Levine added a comment -

            Merged.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Merged.
            Hide
            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:

                  Summary Panel