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

Port meas_modelfit to log package

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_modelfit
    • Labels:
      None

      Description

      Switch from using pex_logging to log in meas_modelfit.

        Attachments

          Issue Links

            Activity

            hchiang2 Hsin-Fang Chiang created issue -
            hchiang2 Hsin-Fang Chiang made changes -
            Field Original Value New Value
            Component/s meas_modelfit [ 11411 ]
            hchiang2 Hsin-Fang Chiang made changes -
            Remote Link This issue links to "Page (Confluence)" [ 14404 ]
            hchiang2 Hsin-Fang Chiang made changes -
            Link This issue blocks DM-8365 [ DM-8365 ]
            Hide
            swinbank John Swinbank added a comment -

            Frossie Economou requests that we tackle this within the first couple of months of S17.

            Show
            swinbank John Swinbank added a comment - Frossie Economou requests that we tackle this within the first couple of months of S17.
            swinbank John Swinbank made changes -
            Epic Link DM-8136 [ 27591 ]
            swinbank John Swinbank made changes -
            Team Data Release Production [ 10301 ]
            swinbank John Swinbank made changes -
            Story Points 2
            swinbank John Swinbank made changes -
            Sprint DRP S17-1 [ 303 ]
            swinbank John Swinbank made changes -
            Assignee Perry Gee [ pgee ]
            pgee Perry Gee made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            pgee Perry Gee added a comment -

            Comments about the use of the fine grained trace debugging are with DM-8356.

            Since meas_modelfit uses the old debug<n> template for n = 10, 8, and 7, I have translated those into trace levels 5,4, and 3. They can be enabled using lsst.log.utils.traceSetAt(component_name, level). All traces <= level are enabled by this method.

            The old pex_logging enabling mechanism was to call lsst.pex.logging.Debug(name, n) with n = 1, ..., 10. This method sets the debug level for name and anything higher on the hierarchy. For examble, lsst.pex.logging.Debug("meas.modelfit.optimizer", 10) seems to enable not only that level, but also "meas.modelfit.optimizer.Optimizer". I don't think that traceSetAt() has this same ability to set up the tree.

            Hsin-Fang Chiang, can you confirm that this is true, that the trace for the individual level must be set individually?

            Show
            pgee Perry Gee added a comment - Comments about the use of the fine grained trace debugging are with DM-8356 . Since meas_modelfit uses the old debug<n> template for n = 10, 8, and 7, I have translated those into trace levels 5,4, and 3. They can be enabled using lsst.log.utils.traceSetAt(component_name, level). All traces <= level are enabled by this method. The old pex_logging enabling mechanism was to call lsst.pex.logging.Debug(name, n) with n = 1, ..., 10. This method sets the debug level for name and anything higher on the hierarchy. For examble, lsst.pex.logging.Debug("meas.modelfit.optimizer", 10) seems to enable not only that level, but also "meas.modelfit.optimizer.Optimizer". I don't think that traceSetAt() has this same ability to set up the tree. Hsin-Fang Chiang , can you confirm that this is true, that the trace for the individual level must be set individually?
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            log can handle the logger hierarchy too, with or without the fine grained trace debugging scheme. For example, lsst.log.utils.traceSetAt("afw.math",4) enables all of its descendant loggers up to TRACE4 as well, unless specified otherwise.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - log can handle the logger hierarchy too, with or without the fine grained trace debugging scheme. For example, lsst.log.utils.traceSetAt("afw.math",4) enables all of its descendant loggers up to TRACE4 as well, unless specified otherwise.
            Hide
            pgee Perry Gee added a comment -

            Do you mind doing this review? You may be one of the only people who knows this code.

            You are correct that the setTraceAt applies to the entire tree. Strange, but I'm sure I tested it. Might have been before I knew about some of the other gotchas.

            Show
            pgee Perry Gee added a comment - Do you mind doing this review? You may be one of the only people who knows this code. You are correct that the setTraceAt applies to the entire tree. Strange, but I'm sure I tested it. Might have been before I knew about some of the other gotchas.
            pgee Perry Gee made changes -
            Reviewers Hsin-Fang Chiang [ hchiang2 ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            pgee Perry Gee added a comment -

            This mistakenly got pushed into "In Review"

            Show
            pgee Perry Gee added a comment - This mistakenly got pushed into "In Review"
            pgee Perry Gee made changes -
            Status In Review [ 10004 ] In Progress [ 3 ]
            Hide
            pgee Perry Gee added a comment -

            This is ready to review under tickets/DM-8357

            Show
            pgee Perry Gee added a comment - This is ready to review under tickets/ DM-8357
            pgee Perry Gee made changes -
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            In terms of usage, this patch looks fine to me. Minor comments on the PR.

            As mentioned in DM-8356, I'm not sure if meas_modelfit really needs to use the fine-level tracing scheme. Each logger of its unique name uses up to two debug levels, so using simply DEBUG and TRACE could be enough too. But I guess the intent may be to add more debug messages in the future using more fine levels? I'll leave the decision to you and Jim Bosch.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - In terms of usage, this patch looks fine to me. Minor comments on the PR. As mentioned in DM-8356 , I'm not sure if meas_modelfit really needs to use the fine-level tracing scheme. Each logger of its unique name uses up to two debug levels, so using simply DEBUG and TRACE could be enough too. But I guess the intent may be to add more debug messages in the future using more fine levels? I'll leave the decision to you and Jim Bosch .
            hchiang2 Hsin-Fang Chiang made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            pgee Perry Gee made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            Hide
            jbosch Jim Bosch added a comment -

            I don't anticipate turning on this logging in production code, so Hsin-Fang Chiang may be right; I'm not sure about the implications of that change to really know. It is important that we maintain the ability to turn on or off any of the name+level combinations that currently exist without turning off the others. It is not important that these have the same name or level, or that levels across different names are kept consistent.

            Show
            jbosch Jim Bosch added a comment - I don't anticipate turning on this logging in production code, so Hsin-Fang Chiang may be right; I'm not sure about the implications of that change to really know. It is important that we maintain the ability to turn on or off any of the name+level combinations that currently exist without turning off the others. It is not important that these have the same name or level, or that levels across different names are kept consistent.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                hchiang2 Hsin-Fang Chiang
                Reviewers:
                Hsin-Fang Chiang
                Watchers:
                Hsin-Fang Chiang, Jim Bosch, John Swinbank, Perry Gee
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel