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

            Activity

            No builds found.
            mfisherlevine Merlin Fisher-Levine created issue -
            mfisherlevine Merlin Fisher-Levine made changes -
            Field Original Value New Value
            Epic Link DM-12733 [ 36332 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue relates to DM-5082 [ DM-5082 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue has to be done after DM-13292 [ DM-13292 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue has to be done before DM-13294 [ DM-13294 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue relates to DM-14190 [ DM-14190 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Risk Score 0
            mfisherlevine Merlin Fisher-Levine made changes -
            Reviewers Russell Owen [ rowen ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Status In Review [ 10004 ] In Review [ 10004 ]
            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.).
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue has to be done before DM-15579 [ DM-15579 ]
            wmwood-vasey Michael Wood-Vasey made changes -
            Link This issue relates to DM-15648 [ DM-15648 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue has to be done before DM-15756 [ DM-15756 ]
            yusra Yusra AlSayyad made changes -
            Watchers Merlin Fisher-Levine, Russell Owen [ Merlin Fisher-Levine, Russell Owen ] Christopher Waters, Merlin Fisher-Levine, Russell Owen [ Christopher Waters, Merlin Fisher-Levine, Russell Owen ]
            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.
            rowen Russell Owen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            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.
            mfisherlevine Merlin Fisher-Levine made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            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?
            swinbank John Swinbank made changes -
            Story Points 20
            swinbank John Swinbank made changes -
            Story Points 20
            mfisherlevine Merlin Fisher-Levine made changes -
            Story Points 70
            swinbank John Swinbank made changes -
            Link This issue is duplicated by DM-14396 [ DM-14396 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue is duplicated by DM-14737 [ DM-14737 ]
            mfisherlevine Merlin Fisher-Levine made changes -
            Link This issue is duplicated by DM-10358 [ DM-10358 ]
            tjenness Tim Jenness made changes -
            Link This issue is triggering DM-15509 [ DM-15509 ]

              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.