# Port meas_extensions_convolved from HSC

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
5
• Sprint:
DRP F16-3, DRP F16-4, DRP F16-5
• Team:
Data Release Production

#### Description

HSC has a new measurement extension: meas_extensions_convolved. This performs aperture photometry with the PSF degraded to nominated seeings (similar to how galaxy photometry is commonly done these days).

Relevant HSC tickets are HSC-1395 and HSC-1408.

#### Activity

No builds found.
Paul Price created issue -
Field Original Value New Value
Epic Link DM-5399 [ 23206 ]
 Assignee Paul Price [ price ]
 Assignee Paul Price [ price ]
 Story Points 5
 Sprint DRP F16-3 [ 237 ]
 Epic Link DM-5399 [ 23206 ] DM-7333 [ 26418 ]
 Sprint DRP F16-3 [ 237 ] DRP F16-3, DRP F16-4 [ 237, 246 ]
 Rank Ranked higher
Hide
Tim Jenness added a comment -

Just in case it's not obvious, if this is being added to lsst_distrib, please check that it works on Python 3 and also update the tests for pytest support.

Show
Tim Jenness added a comment - Just in case it's not obvious, if this is being added to lsst_distrib , please check that it works on Python 3 and also update the tests for pytest support.
Hide
Paul Price added a comment -

Nate Lust, would you mind reviewing this, please? There are a lot of changes to meas_extensions_convolved, but since these are changes from a C++-based measurement algorithm in HSC to a python-based measurement plugin in LSST, I suggest you review the current state. There are also changes to meas_extensions_photometryKron and meas_algorithms.

 price@price-laptop:~/LSST/meas/algorithms (tickets/DM-6784=) $git sub commit 8859ba315f5d1b068d02f10dba3bb777cd716098 Author: Paul Price  Date: Wed Sep 21 16:27:37 2016 -0400    make aperture correction failures optionally non-fatal    Some measurement algorithms may always fail to produce a result on  certain data (e.g., seeing is too good or too bad to make the desired  measurement). The failure to measure an aperture correction for such  a measurement algorithm should not cause an exception. We now allow  certain pre-specified measurement algorithms (through the new config  parameter 'allowFailure') to fail and produce a warning rather than  an exception.    python/lsst/meas/algorithms/measureApCorr.py | 19 +++++++++++++------  tests/testMeasureApCorr.py | 3 +++  2 files changed, 16 insertions(+), 6 deletions(-)     price@price-laptop:~/LSST/meas/extensions/photometryKron (tickets/DM-6784=)$ git sub commit 4003c31208970ced5325aa55b96795a6c4252f69 Author: Paul Price  Date: Mon Jun 27 22:58:05 2016 -0400    make KronAperture public    This will allow other modules to do Kron photometry.    include/lsst/meas/extensions/photometryKron.h | 67 +++++++++++++  .../lsst/meas/extensions/photometryKron/kronLib.i | 3 +  src/KronPhotometry.cc | 107 +++++++--------------  3 files changed, 103 insertions(+), 74 deletions(-)   commit f798a8a61639a3e727c33df62eb3e538394176b1 Author: Paul Price  Date: Thu Sep 15 16:30:01 2016 -0400    Remove unused config parameter 'fixed'    include/lsst/meas/extensions/photometryKron.h | 2 --  1 file changed, 2 deletions(-)   commit 6e814988d11bc532f1a1dfc9448a38aedbe7ef27 Author: Paul Price  Date: Wed Sep 21 11:02:26 2016 -0400    store symbols for wrapped algorithm    So external packages can use them without getting them from the  registry.    python/lsst/meas/extensions/photometryKron/__init__.py | 10 ++++++++--  1 file changed, 8 insertions(+), 2 deletions(-)     price@price-laptop:~/LSST/meas/extensions/convolved (tickets/DM-6784=) $git subcommit 2bc847650b92a466d2a6f072bd867fd1e547d19d Author: Paul Price  Date: Thu Sep 22 14:18:04 2016 -0400    convert to modern measurement framework    This allows us to write the measurement algorithm ("plugin") in python,  since all the heavy lifting is done using other packages. Deleted all  the C++ code and replaced with python implementation that works with the  modern measurement framework.    include/lsst/meas/extensions/convolved.h | 51 ---  lib/SConscript | 3 -  python/lsst/meas/extensions/convolved/SConscript | 3 -  python/lsst/meas/extensions/convolved/__init__.py | 8 +-  python/lsst/meas/extensions/convolved/convolved.py | 503 +++++++++++++++++++++  .../lsst/meas/extensions/convolved/convolvedLib.i | 51 ---  src/convolved.cc | 353 ---------------  7 files changed, 505 insertions(+), 467 deletions(-)   commit abb0f388e8bd35f9cc236d2a9e1d7c6f60ac417e Author: Paul Price  Date: Thu Sep 22 14:20:34 2016 -0400    update and rename test    The test has been updated to allow use with pytest, and renamed following  the desired naming scheme.    Also added tests for the Kron (previously ignored).    tests/convolved.py | 144 -------------------------------------  tests/testConvolved.py | 192 +++++++++++++++++++++++++++++++++++++++++++++++++  2 files changed, 192 insertions(+), 144 deletions(-)   commit 0c7fb7df459242d027323b76ffee3a0896aaf57a Author: Paul Price  Date: Thu Sep 22 14:21:11 2016 -0400    .gitignore: add directory generated by test framework    .gitignore | 1 +  1 file changed, 1 insertion(+)  Show Paul Price added a comment - Nate Lust , would you mind reviewing this, please? There are a lot of changes to meas_extensions_convolved, but since these are changes from a C++-based measurement algorithm in HSC to a python-based measurement plugin in LSST, I suggest you review the current state. There are also changes to meas_extensions_photometryKron and meas_algorithms. price@price-laptop:~/LSST/meas/algorithms (tickets/DM-6784=)$ git sub commit 8859ba315f5d1b068d02f10dba3bb777cd716098 Author: Paul Price <price@astro.princeton.edu> Date: Wed Sep 21 16:27:37 2016 -0400   make aperture correction failures optionally non-fatal Some measurement algorithms may always fail to produce a result on certain data (e.g., seeing is too good or too bad to make the desired measurement). The failure to measure an aperture correction for such a measurement algorithm should not cause an exception. We now allow certain pre-specified measurement algorithms (through the new config parameter 'allowFailure') to fail and produce a warning rather than an exception.   python/lsst/meas/algorithms/measureApCorr.py | 19 +++++++++++++------ tests/testMeasureApCorr.py | 3 +++ 2 files changed, 16 insertions(+), 6 deletions(-)     price@price-laptop:~/LSST/meas/extensions/photometryKron (tickets/DM-6784=) $git sub commit 4003c31208970ced5325aa55b96795a6c4252f69 Author: Paul Price <price@astro.princeton.edu> Date: Mon Jun 27 22:58:05 2016 -0400 make KronAperture public This will allow other modules to do Kron photometry. include/lsst/meas/extensions/photometryKron.h | 67 +++++++++++++ .../lsst/meas/extensions/photometryKron/kronLib.i | 3 + src/KronPhotometry.cc | 107 +++++++-------------- 3 files changed, 103 insertions(+), 74 deletions(-) commit f798a8a61639a3e727c33df62eb3e538394176b1 Author: Paul Price <price@astro.princeton.edu> Date: Thu Sep 15 16:30:01 2016 -0400 Remove unused config parameter 'fixed' include/lsst/meas/extensions/photometryKron.h | 2 -- 1 file changed, 2 deletions(-) commit 6e814988d11bc532f1a1dfc9448a38aedbe7ef27 Author: Paul Price <price@astro.princeton.edu> Date: Wed Sep 21 11:02:26 2016 -0400 store symbols for wrapped algorithm So external packages can use them without getting them from the registry. python/lsst/meas/extensions/photometryKron/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) price@price-laptop:~/LSST/meas/extensions/convolved (tickets/DM-6784=)$ git subcommit 2bc847650b92a466d2a6f072bd867fd1e547d19d Author: Paul Price <price@astro.princeton.edu> Date: Thu Sep 22 14:18:04 2016 -0400   convert to modern measurement framework This allows us to write the measurement algorithm ("plugin") in python, since all the heavy lifting is done using other packages. Deleted all the C++ code and replaced with python implementation that works with the modern measurement framework.   include/lsst/meas/extensions/convolved.h | 51 --- lib/SConscript | 3 - python/lsst/meas/extensions/convolved/SConscript | 3 - python/lsst/meas/extensions/convolved/__init__.py | 8 +- python/lsst/meas/extensions/convolved/convolved.py | 503 +++++++++++++++++++++ .../lsst/meas/extensions/convolved/convolvedLib.i | 51 --- src/convolved.cc | 353 --------------- 7 files changed, 505 insertions(+), 467 deletions(-)   commit abb0f388e8bd35f9cc236d2a9e1d7c6f60ac417e Author: Paul Price <price@astro.princeton.edu> Date: Thu Sep 22 14:20:34 2016 -0400   update and rename test The test has been updated to allow use with pytest, and renamed following the desired naming scheme. Also added tests for the Kron (previously ignored).   tests/convolved.py | 144 ------------------------------------- tests/testConvolved.py | 192 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 144 deletions(-)   commit 0c7fb7df459242d027323b76ffee3a0896aaf57a Author: Paul Price <price@astro.princeton.edu> Date: Thu Sep 22 14:21:11 2016 -0400   .gitignore: add directory generated by test framework   .gitignore | 1 + 1 file changed, 1 insertion(+)
 Reviewers Nate Lust [ nlust ] Status To Do [ 10001 ] In Review [ 10004 ]
Hide
Tim Jenness added a comment -

Paul Price it won't build with lsstsw rebuild for me so can you please add it to the repos.yaml file (or I'm doing something wrong)? Also, it needs to be tested on Python 3 (see my earlier comment on Python 3 and pytest).

Show
Tim Jenness added a comment - Paul Price it won't build with lsstsw rebuild for me so can you please add it to the repos.yaml file (or I'm doing something wrong)? Also, it needs to be tested on Python 3 (see my earlier comment on Python 3 and pytest).
Hide
Paul Price added a comment -

Sorry Tim Jenness. I'm in the process of adding it to lsstsw, and then I'll test it with Python 3. Nate said he's not going to get to this for a few days, so this shouldn't block him.

Show
Paul Price added a comment - Sorry Tim Jenness . I'm in the process of adding it to lsstsw, and then I'll test it with Python 3. Nate said he's not going to get to this for a few days, so this shouldn't block him.
Hide
Paul Price added a comment -

I removed one commit in meas_extensions_photometryKron that was ill-advised and breaking the build.

Show
Paul Price added a comment - It builds under py3 . I removed one commit in meas_extensions_photometryKron that was ill-advised and breaking the build.
 Sprint DRP F16-3, DRP F16-4 [ 237, 246 ] DRP F16-3, DRP F16-4, DRP F16-5 [ 237, 246, 252 ]
 Rank Ranked higher
Hide
Nate Lust added a comment -

I made a few comments on some pull requests, some that require a bit of discussion, but nothing I think will be big show stoppers, after some discussion I will mark this reviewed.

Show
Nate Lust added a comment - I made a few comments on some pull requests, some that require a bit of discussion, but nothing I think will be big show stoppers, after some discussion I will mark this reviewed.
Hide
Paul Price added a comment -

I've pushed some fixups commits for meas_extensions_photometryKron and meas_extensions_convolved that I believe address the issues raised on the GitHub PRs.

Show
Paul Price added a comment - I've pushed some fixups commits for meas_extensions_photometryKron and meas_extensions_convolved that I believe address the issues raised on the GitHub PRs.
Hide
Nate Lust added a comment -

I am satisfied you addressed or considered all the comments, and provided build tests pass feel free to merge

Show
Nate Lust added a comment - I am satisfied you addressed or considered all the comments, and provided build tests pass feel free to merge
 Status In Review [ 10004 ] Reviewed [ 10101 ]
Hide
Tim Jenness added a comment -

Please also add it to lsst_py3 meta package when done to ensure that we can check it on python 3.

Show
Tim Jenness added a comment - Please also add it to lsst_py3 meta package when done to ensure that we can check it on python 3.
Hide
Paul Price added a comment -

Jenkins passed under py2 and py3. Rebasing away the fixups, and will then merge.

Show
Paul Price added a comment - Jenkins passed under py2 and py3 . Rebasing away the fixups, and will then merge.
 Component/s meas_algorithms [ 10732 ]
 Component/s meas_extensions_photometryKron [ 12318 ]
Hide
Paul Price added a comment -

Merged to master.

Thanks, all!

Show
Paul Price added a comment - Merged to master. Added package to lsst_py3. Thanks, all!
 Resolution Done [ 10000 ] Status Reviewed [ 10101 ] Done [ 10002 ]
 Link This issue blocks DM-7889 [ DM-7889 ]
Hide
John Swinbank added a comment -

I don't see docs or a README in this package – surely it can't be marked as "done"?

Show
John Swinbank added a comment - I don't see docs or a README in this package – surely it can't be marked as "done"?
 Link This issue relates to DM-7896 [ DM-7896 ]
Hide
John Swinbank added a comment -
Show
John Swinbank added a comment - DM-7896 .
Hide
Paul Price added a comment -

I don't see docs or a README in this package – surely it can't be marked as "done"?

You must be new here....

Show
Paul Price added a comment - I don't see docs or a README in this package – surely it can't be marked as "done"? You must be new here....

#### People

Assignee:
Paul Price
Reporter:
Paul Price
Reviewers:
Nate Lust
Watchers:
John Swinbank, Nate Lust, Paul Price, Tim Jenness