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

Add TE1 and TE2 KPMs to validate_drp using code developed in DM-3040

    Details

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

      Description

      In Summer 2015, Bob Armstrong wrote code to calculate the residual ellipticity KPM: TE1 and TE2

      https://jira.lsstcorp.org/browse/DM-3040

      Add this code to add TE1 and TE2 to the KPMs calculated in validate_drp

        Attachments

          Issue Links

            Activity

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - https://jira.lsstcorp.org/secure/attachment/27149/create_plot.py
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            Example plots and JSON file that include TE1, TE2 ellipticity measurements.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited Example plots and JSON file that include TE1, TE2 ellipticity measurements.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Removed from review.
            Requires treecorr package, which is not available in the stack.
            The correlation calculation should be redone to not require treecorr.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Removed from review. Requires treecorr package, which is not available in the stack. The correlation calculation should be redone to not require treecorr .
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            treecorr has been added to the available modules in the stack and to the dependencies for validate_drp. DM-9961.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - treecorr has been added to the available modules in the stack and to the dependencies for validate_drp . DM-9961 .
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            Had to add a fix to DM-9961 to actually get treecorr deployed in the PYTHONPATH and PATH.

            Although the corr2 and corr3 executables aren't currently used it seemed best to include them as that would be equivalent to a standard full install of treecorr which is likely what users will be expecting.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited Had to add a fix to DM-9961 to actually get treecorr deployed in the PYTHONPATH and PATH . Although the corr2 and corr3 executables aren't currently used it seemed best to include them as that would be equivalent to a standard full install of treecorr which is likely what users will be expecting.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            treecorr has now been added to the stack modules so I would like to re-activate this review for adding the ellipticity KPMs.

            Jenkins job for tickets/DM-8951 passes:

            https://ci.lsst.codes/job/stack-os-matrix/25666/label=centos-7,python=py3/console
            (The centos-6, python2; centos-7, python2; and osx, python3 builds also passed.)

            The key thing in this Jenkins job is the passing lsst_ci, which exercises validate_drp. The demo job, which produces all of the console output, does not.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - treecorr has now been added to the stack modules so I would like to re-activate this review for adding the ellipticity KPMs. Jenkins job for tickets/ DM-8951 passes: https://ci.lsst.codes/job/stack-os-matrix/25666/label=centos-7,python=py3/console (The centos-6, python2; centos-7, python2; and osx, python3 builds also passed.) The key thing in this Jenkins job is the passing lsst_ci , which exercises validate_drp . The demo job, which produces all of the console output, does not.
            Hide
            Parejkoj John Parejko added a comment -

            Looks like you need to make the PR for the treecorr package? Looks like there's only one minor change there, to which my only comment is whether we want to envPrepend or envAppend (and I think this is a general question, not just one for this package).

            Show
            Parejkoj John Parejko added a comment - Looks like you need to make the PR for the treecorr package? Looks like there's only one minor change there, to which my only comment is whether we want to envPrepend or envAppend (and I think this is a general question, not just one for this package).
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            PR created.
            https://github.com/lsst/treecorr/pull/2
            Agreed that we should revisit envPrepend vs envAppend in a general sense beyond this ticket.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited PR created. https://github.com/lsst/treecorr/pull/2 Agreed that we should revisit envPrepend vs envAppend in a general sense beyond this ticket.
            Hide
            tjenness Tim Jenness added a comment -

            DM-868 is relevant to this discussion. We overwhelmingly use envPrepend.

            Show
            tjenness Tim Jenness added a comment - DM-868 is relevant to this discussion. We overwhelmingly use envPrepend .
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Thanks Tim Jenness for the link to settled practice for envPrepend.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Thanks Tim Jenness for the link to settled practice for envPrepend .
            Hide
            Parejkoj John Parejko added a comment -

            See comments in the PR.

            Show
            Parejkoj John Parejko added a comment - See comments in the PR.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            John Parejko Could you take another look at this PR? Specifically, take a look at the tests I've added in tests/test_tex.py and double-check that I didn't write anything stupid in tests/test_util.py, which currently just checks the ellipticity calculation.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - John Parejko Could you take another look at this PR? Specifically, take a look at the tests I've added in tests/test_tex.py and double-check that I didn't write anything stupid in tests/test_util.py , which currently just checks the ellipticity calculation.
            Hide
            Parejkoj John Parejko added a comment -

            I've added some comments on the new code. There are still some open questions from before as well. Thanks for expanding the tests!

            Show
            Parejkoj John Parejko added a comment - I've added some comments on the new code. There are still some open questions from before as well. Thanks for expanding the tests!
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Thanks, John Parejko, for all of the review work.

            Merged to master.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Thanks, John Parejko , for all of the review work. Merged to master.

              People

              • Assignee:
                wmwood-vasey Michael Wood-Vasey
                Reporter:
                wmwood-vasey Michael Wood-Vasey
                Reviewers:
                John Parejko
                Watchers:
                John Parejko, Michael Wood-Vasey, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel