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

Pull distEst package into obs_subaru

    XMLWordPrintable

    Details

    • Story Points:
      6
    • Sprint:
      Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4, Science Pipelines DM-S15-5, Science Pipelines DM-S15-6
    • Team:
      Data Release Production

      Description

      Reducing HSC data requires an estimate of the distortion, which is provided by the HSC package distEst. This can be pulled into obs_subaru to consolidate code and reduce dependencies.

      I propose to treat distEst as legacy code, which means I will pull it into obs_subaru without major changes to the code style.

        Attachments

          Issue Links

            Activity

            No builds found.
            price Paul Price created issue -
            price Paul Price made changes -
            Field Original Value New Value
            Link This issue relates to DM-245 [ DM-245 ]
            Hide
            price Paul Price added a comment -

            Being related to DM-245, this issue should therefore be understood as being part of the same Epic.

            Show
            price Paul Price added a comment - Being related to DM-245 , this issue should therefore be understood as being part of the same Epic.
            price Paul Price made changes -
            Epic Link DM-1382 [ 14466 ]
            price Paul Price made changes -
            Labels HSC distortion legacy
            Hide
            price Paul Price added a comment -

            I've done this, but am holding off requesting a review until DM-245 is done, since it is built on top of u/lauren/DM-245.

            commit 3590678c6e66fbab61aadba9ed66c7a74577ed73
            Merge: de069f3 eea7699
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 12:14:39 2015 -0500
             
                Merge distEst package into obs_subaru (DM-1794)
                
                Pulled distEst from hsc-repo.mtk.nao.ac.jp:repositories/distEst.git
                
                Conflicts:
                    .gitignore
                    SConstruct
                    lib/SConscript
                    tests/SConscript
             
            commit 8c6decae1efa8da8ea70acf4c573b0db3f5603d5
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 12:39:37 2015 -0500
             
                Reorganise files following merge of distEst.
             
             include/hsc/meas/match/LeastSquares.h   |  18 ----
             include/hsc/meas/match/distest.h        |  22 -----
             include/hsc/meas/match/distest2.h       |  17 ----
             include/hsc/meas/match/distest_utils2.h |  32 -------
             include/lsst/obs/hsc/LeastSquares.h     |  18 ++++
             include/lsst/obs/hsc/distest.h          |  22 +++++
             include/lsst/obs/hsc/distest2.h         |  17 ++++
             include/lsst/obs/hsc/distest_utils2.h   |  32 +++++++
             python/hsc/__init__.py                  |   2 -
             python/hsc/meas/__init__.py             |   2 -
             python/hsc/meas/match/SConscript        |   3 -
             python/hsc/meas/match/__init__.py       |   0
             python/hsc/meas/match/distest.i         |  40 --------
             python/hsc/meas/match/hscDistortion.py  |  56 -----------
             python/lsst/obs/hsc/distest.i           |  40 ++++++++
             src/distest_HSCSIM.cc                   | 164 --------------------------------
             16 files changed, 129 insertions(+), 356 deletions(-)
             
            commit 1f634ec943812b4aaa5f34848a7356f25065b1e5
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 12:50:02 2015 -0500
             
                Prepare distEst for building within obs_subaru.
                
                Changed namespace from hsc::meas::match to lsst::obs::hsc.  Moved
                code that wasn't in any namespace into the lsst::obs::hsc namespace.
             
             include/lsst/obs/hsc/LeastSquares.h   | 15 +++---
             include/lsst/obs/hsc/distest.h        | 22 ++++-----
             include/lsst/obs/hsc/distest2.h       | 11 ++++-
             include/lsst/obs/hsc/distest_utils2.h | 10 +++-
             python/lsst/obs/hsc/SConscript        |  6 ---
             python/lsst/obs/hsc/distest.i         | 14 +++---
             python/lsst/obs/hsc/hscLib.i          | 23 ---------
             src/DistEstXYTransform.cc             |  8 ++--
             src/LeastSquares.cc                   |  8 ++++
             src/distest.cc                        | 10 ++--
             src/distest2.cc                       | 14 ++++++
             src/distest_utils2.cc                 | 87 ++++-------------------------------
             ups/distEst.build                     |  2 -
             ups/distEst.cfg                       | 15 ------
             ups/distEst.table                     |  8 ----
             15 files changed, 79 insertions(+), 174 deletions(-)
             
            commit afccc5f30fc7b914e9a73c7f4535a00e93a4481e
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 14:59:53 2015 -0500
             
                hsc: Fix dependency in building defects
                
                Using the python scripts to build defects requires that the
                SWIG module has been built first.
             
             hsc/SConscript | 6 ++++--
             1 file changed, 4 insertions(+), 2 deletions(-)
             
            commit faf14f39840d7d8433ec99c97704457cbfbfcb68
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 15:02:09 2015 -0500
             
                Updates to allow package to build with LSST stack
             
             include/lsst/obs/subaru/DistEstXYTransform.h |   1 +
             src/distest2.cc                              | 411 ++++-----------------------
             src/distest_utils2.cc                        |   4 +-
             3 files changed, 52 insertions(+), 364 deletions(-)

            Show
            price Paul Price added a comment - I've done this, but am holding off requesting a review until DM-245 is done, since it is built on top of u/lauren/ DM-245 . commit 3590678c6e66fbab61aadba9ed66c7a74577ed73 Merge: de069f3 eea7699 Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 12:14:39 2015 -0500   Merge distEst package into obs_subaru (DM-1794) Pulled distEst from hsc-repo.mtk.nao.ac.jp:repositories/distEst.git Conflicts: .gitignore SConstruct lib/SConscript tests/SConscript   commit 8c6decae1efa8da8ea70acf4c573b0db3f5603d5 Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 12:39:37 2015 -0500   Reorganise files following merge of distEst.   include/hsc/meas/match/LeastSquares.h | 18 ---- include/hsc/meas/match/distest.h | 22 ----- include/hsc/meas/match/distest2.h | 17 ---- include/hsc/meas/match/distest_utils2.h | 32 ------- include/lsst/obs/hsc/LeastSquares.h | 18 ++++ include/lsst/obs/hsc/distest.h | 22 +++++ include/lsst/obs/hsc/distest2.h | 17 ++++ include/lsst/obs/hsc/distest_utils2.h | 32 +++++++ python/hsc/__init__.py | 2 - python/hsc/meas/__init__.py | 2 - python/hsc/meas/match/SConscript | 3 - python/hsc/meas/match/__init__.py | 0 python/hsc/meas/match/distest.i | 40 -------- python/hsc/meas/match/hscDistortion.py | 56 ----------- python/lsst/obs/hsc/distest.i | 40 ++++++++ src/distest_HSCSIM.cc | 164 -------------------------------- 16 files changed, 129 insertions(+), 356 deletions(-)   commit 1f634ec943812b4aaa5f34848a7356f25065b1e5 Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 12:50:02 2015 -0500   Prepare distEst for building within obs_subaru. Changed namespace from hsc::meas::match to lsst::obs::hsc. Moved code that wasn't in any namespace into the lsst::obs::hsc namespace.   include/lsst/obs/hsc/LeastSquares.h | 15 +++--- include/lsst/obs/hsc/distest.h | 22 ++++----- include/lsst/obs/hsc/distest2.h | 11 ++++- include/lsst/obs/hsc/distest_utils2.h | 10 +++- python/lsst/obs/hsc/SConscript | 6 --- python/lsst/obs/hsc/distest.i | 14 +++--- python/lsst/obs/hsc/hscLib.i | 23 --------- src/DistEstXYTransform.cc | 8 ++-- src/LeastSquares.cc | 8 ++++ src/distest.cc | 10 ++-- src/distest2.cc | 14 ++++++ src/distest_utils2.cc | 87 ++++------------------------------- ups/distEst.build | 2 - ups/distEst.cfg | 15 ------ ups/distEst.table | 8 ---- 15 files changed, 79 insertions(+), 174 deletions(-)   commit afccc5f30fc7b914e9a73c7f4535a00e93a4481e Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 14:59:53 2015 -0500   hsc: Fix dependency in building defects Using the python scripts to build defects requires that the SWIG module has been built first.   hsc/SConscript | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)   commit faf14f39840d7d8433ec99c97704457cbfbfcb68 Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 15:02:09 2015 -0500   Updates to allow package to build with LSST stack   include/lsst/obs/subaru/DistEstXYTransform.h | 1 + src/distest2.cc | 411 ++++----------------------- src/distest_utils2.cc | 4 +- 3 files changed, 52 insertions(+), 364 deletions(-)
            price Paul Price made changes -
            Link This issue is blocked by DM-245 [ DM-245 ]
            Hide
            price Paul Price added a comment -

            Lauren, where does this stand after the merge of DM-245?

            Show
            price Paul Price added a comment - Lauren, where does this stand after the merge of DM-245 ?
            Hide
            lauren Lauren MacArthur added a comment -

            Paul Price I pulled down your branch and merged it into the latest DM-245 branch (it had changed A LOT since your pull for this work). There were quite a few conflicts to resolve, but all seems OK. I've pushed this into a u/lauren/DM-1794 branch, so you can have a look and feel free to finish this ticket off (or let me know if you'd like me to do the merge once you've had a look).

            I also plan to make a new issue along the lines of making sure distEst is getting passed numbers in the proper units (without messing around with the camera.py representation which would result in inconsistencies with the other obs_XXX representations).

            Show
            lauren Lauren MacArthur added a comment - Paul Price I pulled down your branch and merged it into the latest DM-245 branch (it had changed A LOT since your pull for this work). There were quite a few conflicts to resolve, but all seems OK. I've pushed this into a u/lauren/ DM-1794 branch, so you can have a look and feel free to finish this ticket off (or let me know if you'd like me to do the merge once you've had a look). I also plan to make a new issue along the lines of making sure distEst is getting passed numbers in the proper units (without messing around with the camera.py representation which would result in inconsistencies with the other obs_XXX representations).
            Hide
            price Paul Price added a comment -

            I'd like to ask Lauren to review this, but because it's her first time I've added John as a back-up. I don't think it's essential to get this in before S15 starts, so please don't kill yourselves getting this done over the weekend. I don't think I can test this via buildbot, so Lauren would you please double-check it builds with the stack you've been using?

            The code on tickets/DM-1794 contains many commits, but I'm listing only the top 5 below because they're the work that I've done to integrate distEst into obs_subaru. I preserved the history of distEst just in case we have to go digging in the future, but the commit discipline in that package is not up to the current LSST standard. This is one of those rare cases where I don't think you should review every commit, but mostly focus on the finished product.

            price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794 %=) $ git --no-pager log --stat -5
            commit 64d84071652e2737e3e5b255c3fc844252f27d4f
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 15:02:09 2015 -0500
             
                Updates to allow package to build with LSST stack
             
             include/lsst/obs/subaru/DistEstXYTransform.h |   1 +
             src/distest2.cc                              | 411 ++++-----------------------
             src/distest_utils2.cc                        |   4 +-
             3 files changed, 52 insertions(+), 364 deletions(-)
             
            commit 3502650cd2c8f54929c3faeff2fbb16c638a2e8b
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 14:59:53 2015 -0500
             
                hsc: Fix dependency in building defects
                
                Using the python scripts to build defects requires that the
                SWIG module has been built first.
             
             hsc/SConscript | 4 +++-
             1 file changed, 3 insertions(+), 1 deletion(-)
             
            commit c32b147f3917b7cb2bb9dba7117c2f62b2a6b93c
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 12:50:02 2015 -0500
             
                Prepare distEst for building within obs_subaru.
                
                Changed namespace from hsc::meas::match to lsst::obs::hsc.  Moved
                code that wasn't in any namespace into the lsst::obs::hsc namespace.
             
             include/lsst/obs/hsc/LeastSquares.h   | 15 +++---
             include/lsst/obs/hsc/distest.h        | 22 ++++-----
             include/lsst/obs/hsc/distest2.h       | 11 ++++-
             include/lsst/obs/hsc/distest_utils2.h | 10 +++-
             python/lsst/obs/hsc/distest.i         | 14 +++---
             src/DistEstXYTransform.cc             |  8 ++--
             src/LeastSquares.cc                   |  8 ++++
             src/distest.cc                        | 10 ++--
             src/distest2.cc                       | 14 ++++++
             src/distest_utils2.cc                 | 87 ++++-------------------------------
             ups/obs_subaru.table                  |  1 -
             11 files changed, 79 insertions(+), 121 deletions(-)
             
            commit 69197ea5246a3803ea371d6a03846615bd8a803b
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jan 21 12:39:37 2015 -0500
             
                Reorganise files following merge of distEst.
             
             include/hsc/meas/match/LeastSquares.h   |  18 ----
             include/hsc/meas/match/distest.h        |  22 -----
             include/hsc/meas/match/distest2.h       |  17 ----
             include/hsc/meas/match/distest_utils2.h |  32 -------
             include/lsst/obs/hsc/LeastSquares.h     |  18 ++++
             include/lsst/obs/hsc/distest.h          |  22 +++++
             include/lsst/obs/hsc/distest2.h         |  17 ++++
             include/lsst/obs/hsc/distest_utils2.h   |  32 +++++++
             python/hsc/meas/match/distest.i         |  40 --------
             python/hsc/meas/match/hscDistortion.py  |  56 -----------
             python/lsst/obs/hsc/distest.i           |  40 ++++++++
             src/distest_HSCSIM.cc                   | 164 --------------------------------
             12 files changed, 129 insertions(+), 349 deletions(-)
             
            commit a4092c6219affdb54253cfef6a17135cde901d0c
            Merge: 44fc8df 39e3fed
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Fri Feb 27 16:36:56 2015 -0500
             
                Merge distEst package into obs_subaru
                
                Pulled distEst from hsc-repo.mtk.nao.ac.jp:repositories/distEst.git
                
                distEst is a program from the HSC software team for estimating the
                distortion in HSC.  It was its own package because it was originally
                used independently of obs_subaru to create simulated HSC images.
                We're now pulling it into obs_subaru because it no longer has any
                utility as a stand-alone package, and the distortion correction is
                essential for processing HSC data.
             
            price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794 %=) $ git --no-pager diff --stat origin/master...
             example/DISTforSIM.cc                        | 2863 +++++++++++++++++++++++
             example/DISTforSIM.h                         |   33 +
             example/display.py                           |   79 +
             example/distEst2.py                          |   38 +
             example/example.py                           |   23 +
             example/example2.py                          |   26 +
             hsc/SConscript                               |    4 +-
             include/lsst/obs/hsc/LeastSquares.h          |   17 +
             include/lsst/obs/hsc/distest.h               |   16 +
             include/lsst/obs/hsc/distest2.h              |   24 +
             include/lsst/obs/hsc/distest_utils2.h        |   38 +
             include/lsst/obs/subaru/DistEstXYTransform.h |    1 +
             python/lsst/obs/hsc/distest.i                |   40 +
             src/DistEstXYTransform.cc                    |    8 +-
             src/LeastSquares.cc                          |  296 +++
             src/distest.cc                               |  164 ++
             src/distest2.cc                              |  810 +++++++
             src/distest_utils2.cc                        | 3204 ++++++++++++++++++++++++++
             tests/invariant.py                           |  109 +
             ups/obs_subaru.table                         |    1 -
             20 files changed, 7788 insertions(+), 6 deletions(-)

            For posterity, the command I used to prepare distEst for merging was:

            git filter-branch \
                --prune-empty \
                --index-filter '
                    git ls-tree -z -r --name-only --full-tree $GIT_COMMIT \
                    | grep -z -v "^include/" \
                    | grep -z -v "^python/hsc/meas/match/distest.i$" \
                    | grep -z -v "^python/hsc/meas/match/hscDistortion.py$" \
                    | grep -z -v "^src/" \
                    | grep -z -v "^example/" \
                    | grep -z -v "^tests/invariant.py$" \
                    | xargs -0 git rm --cached -r
                ' \
                -- \
                --all

            Then, in obs_subaru I did git fetch /path/to/distEst; git merge FETCH_HEAD. Then I added the above 5 commits to clean things up.

            Show
            price Paul Price added a comment - I'd like to ask Lauren to review this, but because it's her first time I've added John as a back-up. I don't think it's essential to get this in before S15 starts, so please don't kill yourselves getting this done over the weekend. I don't think I can test this via buildbot, so Lauren would you please double-check it builds with the stack you've been using? The code on tickets/ DM-1794 contains many commits, but I'm listing only the top 5 below because they're the work that I've done to integrate distEst into obs_subaru. I preserved the history of distEst just in case we have to go digging in the future, but the commit discipline in that package is not up to the current LSST standard. This is one of those rare cases where I don't think you should review every commit, but mostly focus on the finished product. price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794 %=) $ git --no-pager log --stat -5 commit 64d84071652e2737e3e5b255c3fc844252f27d4f Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 15:02:09 2015 -0500   Updates to allow package to build with LSST stack   include/lsst/obs/subaru/DistEstXYTransform.h | 1 + src/distest2.cc | 411 ++++----------------------- src/distest_utils2.cc | 4 +- 3 files changed, 52 insertions(+), 364 deletions(-)   commit 3502650cd2c8f54929c3faeff2fbb16c638a2e8b Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 14:59:53 2015 -0500   hsc: Fix dependency in building defects Using the python scripts to build defects requires that the SWIG module has been built first.   hsc/SConscript | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)   commit c32b147f3917b7cb2bb9dba7117c2f62b2a6b93c Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 12:50:02 2015 -0500   Prepare distEst for building within obs_subaru. Changed namespace from hsc::meas::match to lsst::obs::hsc. Moved code that wasn't in any namespace into the lsst::obs::hsc namespace.   include/lsst/obs/hsc/LeastSquares.h | 15 +++--- include/lsst/obs/hsc/distest.h | 22 ++++----- include/lsst/obs/hsc/distest2.h | 11 ++++- include/lsst/obs/hsc/distest_utils2.h | 10 +++- python/lsst/obs/hsc/distest.i | 14 +++--- src/DistEstXYTransform.cc | 8 ++-- src/LeastSquares.cc | 8 ++++ src/distest.cc | 10 ++-- src/distest2.cc | 14 ++++++ src/distest_utils2.cc | 87 ++++------------------------------- ups/obs_subaru.table | 1 - 11 files changed, 79 insertions(+), 121 deletions(-)   commit 69197ea5246a3803ea371d6a03846615bd8a803b Author: Paul Price <price@astro.princeton.edu> Date: Wed Jan 21 12:39:37 2015 -0500   Reorganise files following merge of distEst.   include/hsc/meas/match/LeastSquares.h | 18 ---- include/hsc/meas/match/distest.h | 22 ----- include/hsc/meas/match/distest2.h | 17 ---- include/hsc/meas/match/distest_utils2.h | 32 ------- include/lsst/obs/hsc/LeastSquares.h | 18 ++++ include/lsst/obs/hsc/distest.h | 22 +++++ include/lsst/obs/hsc/distest2.h | 17 ++++ include/lsst/obs/hsc/distest_utils2.h | 32 +++++++ python/hsc/meas/match/distest.i | 40 -------- python/hsc/meas/match/hscDistortion.py | 56 ----------- python/lsst/obs/hsc/distest.i | 40 ++++++++ src/distest_HSCSIM.cc | 164 -------------------------------- 12 files changed, 129 insertions(+), 349 deletions(-)   commit a4092c6219affdb54253cfef6a17135cde901d0c Merge: 44fc8df 39e3fed Author: Paul Price <price@astro.princeton.edu> Date: Fri Feb 27 16:36:56 2015 -0500   Merge distEst package into obs_subaru Pulled distEst from hsc-repo.mtk.nao.ac.jp:repositories/distEst.git distEst is a program from the HSC software team for estimating the distortion in HSC. It was its own package because it was originally used independently of obs_subaru to create simulated HSC images. We're now pulling it into obs_subaru because it no longer has any utility as a stand-alone package, and the distortion correction is essential for processing HSC data.   price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794 %=) $ git --no-pager diff --stat origin/master... example/DISTforSIM.cc | 2863 +++++++++++++++++++++++ example/DISTforSIM.h | 33 + example/display.py | 79 + example/distEst2.py | 38 + example/example.py | 23 + example/example2.py | 26 + hsc/SConscript | 4 +- include/lsst/obs/hsc/LeastSquares.h | 17 + include/lsst/obs/hsc/distest.h | 16 + include/lsst/obs/hsc/distest2.h | 24 + include/lsst/obs/hsc/distest_utils2.h | 38 + include/lsst/obs/subaru/DistEstXYTransform.h | 1 + python/lsst/obs/hsc/distest.i | 40 + src/DistEstXYTransform.cc | 8 +- src/LeastSquares.cc | 296 +++ src/distest.cc | 164 ++ src/distest2.cc | 810 +++++++ src/distest_utils2.cc | 3204 ++++++++++++++++++++++++++ tests/invariant.py | 109 + ups/obs_subaru.table | 1 - 20 files changed, 7788 insertions(+), 6 deletions(-) For posterity, the command I used to prepare distEst for merging was: git filter-branch \ --prune-empty \ --index-filter ' git ls-tree -z -r --name-only --full-tree $GIT_COMMIT \ | grep -z -v "^include/" \ | grep -z -v "^python/hsc/meas/match/distest.i$" \ | grep -z -v "^python/hsc/meas/match/hscDistortion.py$" \ | grep -z -v "^src/" \ | grep -z -v "^example/" \ | grep -z -v "^tests/invariant.py$" \ | xargs -0 git rm --cached -r ' \ -- \ --all Then, in obs_subaru I did git fetch /path/to/distEst; git merge FETCH_HEAD . Then I added the above 5 commits to clean things up.
            price Paul Price made changes -
            Reviewers John Swinbank, Lauren MacArthur [ swinbank, lauren ]
            Status To Do [ 10001 ] In Review [ 10004 ]
            Hide
            swinbank John Swinbank added a comment -

            Sorry – JIRA didn't mail me about this, so I've only just seen that you assigned me as a reviewer. Not going to get this done in W15; sorry. I'll bump to the next cycle, and take a look at it shortly.

            Show
            swinbank John Swinbank added a comment - Sorry – JIRA didn't mail me about this, so I've only just seen that you assigned me as a reviewer. Not going to get this done in W15; sorry. I'll bump to the next cycle, and take a look at it shortly.
            swinbank John Swinbank made changes -
            Epic Link DM-1382 [ 14466 ] DM-1907 [ 15927 ]
            swinbank John Swinbank made changes -
            Sprint Science Pipelines DM-S15-1 [ 140 ]
            swinbank John Swinbank made changes -
            Labels HSC distortion legacy distortion legacy
            Hide
            swinbank John Swinbank added a comment -

            I looked this over briefly earlier. I understand the comment that the commit structure isn't what we'd hope for on LSST, but, even given that, my impression was that the code itself could use a lot more work to comply with LSST conventions.

            I'm leery of investing too much time that we've not budgeted for in this code, if it's fundamentally adequate to get HSC data flowing through the LSST pipeline and will only ever be of interest to a minority of folks (most of whom are probably already happily using the HSC version). However, I'm also unhappy with just merging something that's fairly flagrantly violating a bunch of LSST conventions into the LSST repository; I'm not sure we can avoid this by just branding it 'legacy'.

            We discussed this a little on HipChat, and didn't really reach a conclusion. Further thoughts welcome.

            Show
            swinbank John Swinbank added a comment - I looked this over briefly earlier. I understand the comment that the commit structure isn't what we'd hope for on LSST, but, even given that, my impression was that the code itself could use a lot more work to comply with LSST conventions. I'm leery of investing too much time that we've not budgeted for in this code, if it's fundamentally adequate to get HSC data flowing through the LSST pipeline and will only ever be of interest to a minority of folks (most of whom are probably already happily using the HSC version). However, I'm also unhappy with just merging something that's fairly flagrantly violating a bunch of LSST conventions into the LSST repository; I'm not sure we can avoid this by just branding it 'legacy'. We discussed this a little on HipChat, and didn't really reach a conclusion. Further thoughts welcome.
            Hide
            price Paul Price added a comment -

            It's not a great deal of code, cleanup is easier than writing new code, and it would be good to have it cleaned up, so I'll give it a go. Is it worth preserving the history if everything's going to get cleaned up?

            Show
            price Paul Price added a comment - It's not a great deal of code, cleanup is easier than writing new code, and it would be good to have it cleaned up, so I'll give it a go. Is it worth preserving the history if everything's going to get cleaned up?
            Hide
            ktl Kian-Tat Lim added a comment -

            Generally my attitude would be that most obs_ packages should be treated as something like contributions and not necessarily required to meet LSST standards. But if cleaning it up is possible, I think it's better to do it.

            Show
            ktl Kian-Tat Lim added a comment - Generally my attitude would be that most obs_ packages should be treated as something like contributions and not necessarily required to meet LSST standards. But if cleaning it up is possible, I think it's better to do it.
            Hide
            swinbank John Swinbank added a comment -

            Paul Price, if you're willing to spend a bit of time on cleanup that would be great! I wouldn't let this become a significant time sink, though, especially given K-T's comment. Personally, I don't think preserving the HSC history is essential.

            Kian-Tat Lim, that attitude is basically what I was hoping for. But: it seems likely that once we've got most of the HSC changes merged back to LSST, we'll want to use obs_subaru not just for the entertainment of scientists but also as part of integration testing for the LSST stack. Does that change the position?

            Show
            swinbank John Swinbank added a comment - Paul Price , if you're willing to spend a bit of time on cleanup that would be great! I wouldn't let this become a significant time sink, though, especially given K-T's comment. Personally, I don't think preserving the HSC history is essential. Kian-Tat Lim , that attitude is basically what I was hoping for. But: it seems likely that once we've got most of the HSC changes merged back to LSST, we'll want to use obs_subaru not just for the entertainment of scientists but also as part of integration testing for the LSST stack. Does that change the position?
            Hide
            swinbank John Swinbank added a comment -

            We discussed this in today's coordination standup. The consensus is that we should enforce LSST's coding standards for things going into obs_subaru and, indeed, any other packages obs_ which will be a requirement for LSST's testing and development. Other obs_ packages which are judged to not be of direct importance to LSST may be regarded as "contrib" or similar and hence not held to the same standards.

            Since this is work required by LSST, it's of course appropriate for LSST to pay for it – that is, we'd expect this effort to come out of Paul Price's (in this case) LSST time. Paul, I get the impression from your comment above that it's not a huge job, but feel free to create a new story or add more story points to this one to cover the effort you'll be putting in, and do scream if it's something you can't see yourself having time for in the foreseeable future due to your HSC commitments.

            Show
            swinbank John Swinbank added a comment - We discussed this in today's coordination standup. The consensus is that we should enforce LSST's coding standards for things going into obs_subaru and, indeed, any other packages obs_ which will be a requirement for LSST's testing and development. Other obs_ packages which are judged to not be of direct importance to LSST may be regarded as "contrib" or similar and hence not held to the same standards. Since this is work required by LSST, it's of course appropriate for LSST to pay for it – that is, we'd expect this effort to come out of Paul Price 's (in this case) LSST time. Paul, I get the impression from your comment above that it's not a huge job, but feel free to create a new story or add more story points to this one to cover the effort you'll be putting in, and do scream if it's something you can't see yourself having time for in the foreseeable future due to your HSC commitments.
            price Paul Price made changes -
            Story Points 4 6
            swinbank John Swinbank made changes -
            Status In Review [ 10004 ] In Progress [ 3 ]
            swinbank John Swinbank made changes -
            Sprint Science Pipelines DM-S15-1 [ 140 ] Science Pipelines DM-S15-1, Science Pipelines DM-S15-2 [ 140, 151 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            jbosch Jim Bosch made changes -
            Sprint Science Pipelines DM-S15-1, Science Pipelines DM-S15-2 [ 140, 151 ] Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3 [ 140, 151, 155 ]
            jbosch Jim Bosch made changes -
            Rank Ranked higher
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-2796 [ DM-2796 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-2796 [ DM-2796 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-2796 [ DM-2796 ]
            krughoff Simon Krughoff made changes -
            Sprint Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3 [ 140, 151, 155 ] Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4 [ 140, 151, 155, 159 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            lauren Lauren MacArthur made changes -
            Link This issue is duplicated by DM-2796 [ DM-2796 ]
            swinbank John Swinbank made changes -
            Link This issue relates to DM-2796 [ DM-2796 ]
            jbosch Jim Bosch made changes -
            Labels distortion legacy HSC distortion legacy
            swinbank John Swinbank made changes -
            Sprint Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4 [ 140, 151, 155, 159 ] Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4, Science Pipelines DM-S15-5 [ 140, 151, 155, 159, 162 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            swinbank John Swinbank made changes -
            Link This issue is duplicated by DM-2194 [ DM-2194 ]
            Hide
            price Paul Price added a comment -

            I think I've broken the back of this... Hope to have it polished up soon.

            Show
            price Paul Price added a comment - I think I've broken the back of this... Hope to have it polished up soon.
            Hide
            price Paul Price added a comment -

            John Swinbank, would you please have a look at this? Lauren MacArthur, you may want to look too, but I realise I just gave you a different review.

            There are several commits on tickets/DM-1794 of obs_subaru. Most are fixes of little things that I noticed, but the big one is 111d2ea, which imports the functionality of distEst into obs_subaru.

            price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794=) $ git sub
            commit 59f92bb9cd081b79b52dd3f7eee96c1c2560a57c
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:53:55 2015 -0400
             
                genDefectFits.py: fix instantiation of mapper
             
             bin/genDefectFits.py | 2 +-
             1 file changed, 1 insertion(+), 1 deletion(-)
             
            commit a2780b563992a64a5d51537d620fde62bf2355e5
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:54:32 2015 -0400
             
                registryInfo.py: don't require postgresql
             
             bin/registryInfo.py | 3 ++-
             1 file changed, 2 insertions(+), 1 deletion(-)
             
            commit 0302788e7feb5473b1be05e5dd093de6ceca410c
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:56:06 2015 -0400
             
                require SWIGed module before building defects registry
             
             hsc/SConscript | 4 +++-
             1 file changed, 3 insertions(+), 1 deletion(-)
             
            commit 555767b59ad08c5e307437c1ed8b7ffcc13065f1
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:03:30 2015 -0400
             
                genCameraRepository.py: convert to a plate scale of 1
                
                LSST likes to use mm/pixels, but Subaru works in nominal pixels from the
                boresite, or a plate scale of 1.  This is true for both Suprime-Cam and
                HSC.
                
                This does not include updates to the camera definition files, just the
                conversion.
             
             bin/genCameraRepository.py | 6 ++----
             1 file changed, 2 insertions(+), 4 deletions(-)
             
            commit 6a7fd49769570367718e0af10ecbd28b8664b92a
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:07:46 2015 -0400
             
                fix amp names
                
                Subaru amplifiers aren't indexed in two dimensions, so no need for an
                extra number that's always zero.
             
             bin/genCameraRepository.py    | 2 +-
             python/lsst/obs/subaru/isr.py | 5 +----
             2 files changed, 2 insertions(+), 5 deletions(-)
             
            commit 111d2ea907e8893557e5ccef9e3e155eb265c4ac
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:15:35 2015 -0400
             
                add HSC distortion model rather than using external package distEst
                
                This implements distEst using the new cameraGeom framework, and activates
                the new distortion model.
             
             bin/genCameraRepository.py                   |   5 +-
             hsc/camera/0_00.fits                         |   4 +-
             hsc/camera/0_01.fits                         |   6 +-
             hsc/camera/0_02.fits                         |   6 +-
             hsc/camera/0_03.fits                         |   4 +-
             hsc/camera/0_04.fits                         |   4 +-
             hsc/camera/0_05.fits                         |   2 +-
             hsc/camera/0_06.fits                         |   6 +-
             hsc/camera/0_07.fits                         |   4 +-
             hsc/camera/0_08.fits                         |   4 +-
             hsc/camera/0_09.fits                         |   4 +-
             hsc/camera/0_10.fits                         |   8 +-
             hsc/camera/0_11.fits                         |   2 +-
             hsc/camera/0_12.fits                         |   4 +-
             hsc/camera/0_13.fits                         |   2 +-
             hsc/camera/0_14.fits                         |   6 +-
             hsc/camera/0_15.fits                         |   2 +-
             hsc/camera/0_16.fits                         |   2 +-
             hsc/camera/0_17.fits                         |   6 +-
             hsc/camera/0_18.fits                         |   4 +-
             hsc/camera/0_19.fits                         |   6 +-
             hsc/camera/0_20.fits                         |   2 +-
             hsc/camera/0_21.fits                         |   2 +-
             hsc/camera/0_22.fits                         |   4 +-
             hsc/camera/0_23.fits                         |   4 +-
             hsc/camera/0_24.fits                         |   4 +-
             hsc/camera/0_25.fits                         |   4 +-
             hsc/camera/0_26.fits                         |   4 +-
             hsc/camera/0_27.fits                         |   2 +-
             hsc/camera/0_28.fits                         |   4 +-
             hsc/camera/0_29.fits                         |   4 +-
             hsc/camera/0_30.fits                         |   2 +-
             hsc/camera/0_31.fits                         |   2 +-
             hsc/camera/0_32.fits                         |   2 +-
             hsc/camera/0_33.fits                         |   4 +-
             hsc/camera/0_34.fits                         |   4 +-
             hsc/camera/0_35.fits                         |   2 +-
             hsc/camera/0_36.fits                         |   4 +-
             hsc/camera/0_37.fits                         |   8 +-
             hsc/camera/0_38.fits                         |   4 +-
             hsc/camera/0_39.fits                         |   2 +-
             hsc/camera/0_40.fits                         |   4 +-
             hsc/camera/0_41.fits                         |   2 +-
             hsc/camera/0_42.fits                         |   6 +-
             hsc/camera/0_43.fits                         |   6 +-
             hsc/camera/0_44.fits                         |   4 +-
             hsc/camera/0_45.fits                         |   2 +-
             hsc/camera/0_46.fits                         |   4 +-
             hsc/camera/0_47.fits                         |   6 +-
             hsc/camera/0_48.fits                         |   4 +-
             hsc/camera/0_51.fits                         |   4 +-
             hsc/camera/0_52.fits                         |   4 +-
             hsc/camera/0_53.fits                         |   6 +-
             hsc/camera/0_54.fits                         |   6 +-
             hsc/camera/0_55.fits                         |   2 +-
             hsc/camera/0_56.fits                         |   2 +-
             hsc/camera/0_57.fits                         |   6 +-
             hsc/camera/1_00.fits                         |   4 +-
             hsc/camera/1_01.fits                         |   2 +-
             hsc/camera/1_02.fits                         |   4 +-
             hsc/camera/1_03.fits                         |   4 +-
             hsc/camera/1_04.fits                         |   4 +-
             hsc/camera/1_05.fits                         |   2 +-
             hsc/camera/1_06.fits                         |   2 +-
             hsc/camera/1_07.fits                         |   4 +-
             hsc/camera/1_08.fits                         |   6 +-
             hsc/camera/1_09.fits                         |   6 +-
             hsc/camera/1_10.fits                         |   2 +-
             hsc/camera/1_11.fits                         |   8 +-
             hsc/camera/1_12.fits                         |   4 +-
             hsc/camera/1_13.fits                         |   4 +-
             hsc/camera/1_14.fits                         |   4 +-
             hsc/camera/1_15.fits                         |   2 +-
             hsc/camera/1_16.fits                         |   6 +-
             hsc/camera/1_17.fits                         |   6 +-
             hsc/camera/1_18.fits                         |   2 +-
             hsc/camera/1_19.fits                         |   2 +-
             hsc/camera/1_20.fits                         |   2 +-
             hsc/camera/1_21.fits                         |   2 +-
             hsc/camera/1_22.fits                         |   2 +-
             hsc/camera/1_23.fits                         |   2 +-
             hsc/camera/1_24.fits                         |   2 +-
             hsc/camera/1_25.fits                         |   2 +-
             hsc/camera/1_26.fits                         |   2 +-
             hsc/camera/1_27.fits                         |   4 +-
             hsc/camera/1_28.fits                         |   4 +-
             hsc/camera/1_29.fits                         |   2 +-
             hsc/camera/1_30.fits                         |   6 +-
             hsc/camera/1_31.fits                         |   6 +-
             hsc/camera/1_32.fits                         |   2 +-
             hsc/camera/1_33.fits                         |   4 +-
             hsc/camera/1_34.fits                         |   2 +-
             hsc/camera/1_35.fits                         |   4 +-
             hsc/camera/1_36.fits                         |   4 +-
             hsc/camera/1_37.fits                         |   2 +-
             hsc/camera/1_38.fits                         |   4 +-
             hsc/camera/1_39.fits                         |   2 +-
             hsc/camera/1_40.fits                         |   2 +-
             hsc/camera/1_41.fits                         |   4 +-
             hsc/camera/1_42.fits                         |   2 +-
             hsc/camera/1_43.fits                         |   4 +-
             hsc/camera/1_44.fits                         |   4 +-
             hsc/camera/1_45.fits                         |   4 +-
             hsc/camera/1_46.fits                         |   2 +-
             hsc/camera/1_47.fits                         |   4 +-
             hsc/camera/1_48.fits                         |   4 +-
             hsc/camera/1_51.fits                         |   6 +-
             hsc/camera/1_52.fits                         |   4 +-
             hsc/camera/1_53.fits                         |   4 +-
             hsc/camera/1_54.fits                         |   4 +-
             hsc/camera/1_55.fits                         |   6 +-
             hsc/camera/1_56.fits                         |   6 +-
             hsc/camera/1_57.fits                         |   6 +-
             hsc/camera/camera.py                         | 940 ++++++++++++++++++++++++++++++------------------------------
             include/lsst/obs/subaru/DistEstXYTransform.h |  62 ----
             include/lsst/obs/subaru/HscDistortion.h      |  91 ++++++
             python/lsst/obs/hsc/transformRegistry.py     | 271 ++++++++++++++++--
             python/lsst/obs/subaru/subaruLib.i           |   6 +-
             src/DistEstXYTransform.cc                    |  63 ----
             src/HscDistortion.cc                         | 139 +++++++++
             tests/hscDistortion.py                       | 119 ++++++++
             ups/obs_subaru.cfg                           |  11 +-
             ups/obs_subaru.table                         |   1 -
             123 files changed, 1294 insertions(+), 840 deletions(-)
             
            commit b6aaa081fdf6b8522274ba94ab7cafe76cad6fa0
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:16:32 2015 -0400
             
                generate new camera definitions for Suprime-Cam
             
             suprimecam/camera/Chihiro.fits    |  4 ++--
             suprimecam/camera/Clarisse.fits   |  6 ++---
             suprimecam/camera/Fio.fits        |  2 +-
             suprimecam/camera/Kiki.fits       |  6 ++---
             suprimecam/camera/Nausicaa.fits   |  6 ++---
             suprimecam/camera/Ponyo.fits      |  4 ++--
             suprimecam/camera/San.fits        |  4 ++--
             suprimecam/camera/Satsuki.fits    |  2 +-
             suprimecam/camera/Sheeta.fits     |  4 ++--
             suprimecam/camera/Sophie.fits     |  6 ++---
             suprimecam/camera/camera.py       | 88 ++++++++++++++++++++++++++++++++++------------------------------------
             suprimecam/mit_camera/camera.py   | 90 ++++++++++++++++++++++++++++++++++--------------------------------------
             suprimecam/mit_camera/si001s.fits |  2 +-
             suprimecam/mit_camera/si002s.fits |  2 +-
             suprimecam/mit_camera/si005s.fits |  2 +-
             suprimecam/mit_camera/si006s.fits |  2 +-
             suprimecam/mit_camera/w4c5.fits   |  2 +-
             suprimecam/mit_camera/w67c1.fits  |  2 +-
             suprimecam/mit_camera/w6c1.fits   |  2 +-
             suprimecam/mit_camera/w7c3.fits   |  2 +-
             suprimecam/mit_camera/w93c2.fits  |  2 +-
             suprimecam/mit_camera/w9c2.fits   |  2 +-
             22 files changed, 117 insertions(+), 125 deletions(-)
             
            commit b9e6c60db15d415a3ed76196d32b2b35017c0242
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:16:58 2015 -0400
             
                cleanup lib/Sconscript
             
             lib/SConscript | 2 --
             1 file changed, 2 deletions(-)
             
            commit eee651a591ca300ca5524f1c5543cbae1b9dd8c3
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:18:03 2015 -0400
             
                add image of CCD positions for HSC
                
                This is useful for debugging.
                
                Taken from http://subarutelescope.org/Observing/Instruments/HSC/CCDPosition_20140811.png
             
             hsc/CCDPosition_20140811.png | Bin 0 -> 370956 bytes
             1 file changed, 0 insertions(+), 0 deletions(-)
            

            Show
            price Paul Price added a comment - John Swinbank , would you please have a look at this? Lauren MacArthur , you may want to look too, but I realise I just gave you a different review. There are several commits on tickets/ DM-1794 of obs_subaru. Most are fixes of little things that I noticed, but the big one is 111d2ea, which imports the functionality of distEst into obs_subaru. price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794=) $ git sub commit 59f92bb9cd081b79b52dd3f7eee96c1c2560a57c Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:53:55 2015 -0400   genDefectFits.py: fix instantiation of mapper   bin/genDefectFits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)   commit a2780b563992a64a5d51537d620fde62bf2355e5 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:54:32 2015 -0400   registryInfo.py: don't require postgresql   bin/registryInfo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)   commit 0302788e7feb5473b1be05e5dd093de6ceca410c Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:56:06 2015 -0400   require SWIGed module before building defects registry   hsc/SConscript | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)   commit 555767b59ad08c5e307437c1ed8b7ffcc13065f1 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:03:30 2015 -0400   genCameraRepository.py: convert to a plate scale of 1 LSST likes to use mm/pixels, but Subaru works in nominal pixels from the boresite, or a plate scale of 1. This is true for both Suprime-Cam and HSC. This does not include updates to the camera definition files, just the conversion.   bin/genCameraRepository.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)   commit 6a7fd49769570367718e0af10ecbd28b8664b92a Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:07:46 2015 -0400   fix amp names Subaru amplifiers aren't indexed in two dimensions, so no need for an extra number that's always zero.   bin/genCameraRepository.py | 2 +- python/lsst/obs/subaru/isr.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-)   commit 111d2ea907e8893557e5ccef9e3e155eb265c4ac Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:15:35 2015 -0400   add HSC distortion model rather than using external package distEst This implements distEst using the new cameraGeom framework, and activates the new distortion model.   bin/genCameraRepository.py | 5 +- hsc/camera/0_00.fits | 4 +- hsc/camera/0_01.fits | 6 +- hsc/camera/0_02.fits | 6 +- hsc/camera/0_03.fits | 4 +- hsc/camera/0_04.fits | 4 +- hsc/camera/0_05.fits | 2 +- hsc/camera/0_06.fits | 6 +- hsc/camera/0_07.fits | 4 +- hsc/camera/0_08.fits | 4 +- hsc/camera/0_09.fits | 4 +- hsc/camera/0_10.fits | 8 +- hsc/camera/0_11.fits | 2 +- hsc/camera/0_12.fits | 4 +- hsc/camera/0_13.fits | 2 +- hsc/camera/0_14.fits | 6 +- hsc/camera/0_15.fits | 2 +- hsc/camera/0_16.fits | 2 +- hsc/camera/0_17.fits | 6 +- hsc/camera/0_18.fits | 4 +- hsc/camera/0_19.fits | 6 +- hsc/camera/0_20.fits | 2 +- hsc/camera/0_21.fits | 2 +- hsc/camera/0_22.fits | 4 +- hsc/camera/0_23.fits | 4 +- hsc/camera/0_24.fits | 4 +- hsc/camera/0_25.fits | 4 +- hsc/camera/0_26.fits | 4 +- hsc/camera/0_27.fits | 2 +- hsc/camera/0_28.fits | 4 +- hsc/camera/0_29.fits | 4 +- hsc/camera/0_30.fits | 2 +- hsc/camera/0_31.fits | 2 +- hsc/camera/0_32.fits | 2 +- hsc/camera/0_33.fits | 4 +- hsc/camera/0_34.fits | 4 +- hsc/camera/0_35.fits | 2 +- hsc/camera/0_36.fits | 4 +- hsc/camera/0_37.fits | 8 +- hsc/camera/0_38.fits | 4 +- hsc/camera/0_39.fits | 2 +- hsc/camera/0_40.fits | 4 +- hsc/camera/0_41.fits | 2 +- hsc/camera/0_42.fits | 6 +- hsc/camera/0_43.fits | 6 +- hsc/camera/0_44.fits | 4 +- hsc/camera/0_45.fits | 2 +- hsc/camera/0_46.fits | 4 +- hsc/camera/0_47.fits | 6 +- hsc/camera/0_48.fits | 4 +- hsc/camera/0_51.fits | 4 +- hsc/camera/0_52.fits | 4 +- hsc/camera/0_53.fits | 6 +- hsc/camera/0_54.fits | 6 +- hsc/camera/0_55.fits | 2 +- hsc/camera/0_56.fits | 2 +- hsc/camera/0_57.fits | 6 +- hsc/camera/1_00.fits | 4 +- hsc/camera/1_01.fits | 2 +- hsc/camera/1_02.fits | 4 +- hsc/camera/1_03.fits | 4 +- hsc/camera/1_04.fits | 4 +- hsc/camera/1_05.fits | 2 +- hsc/camera/1_06.fits | 2 +- hsc/camera/1_07.fits | 4 +- hsc/camera/1_08.fits | 6 +- hsc/camera/1_09.fits | 6 +- hsc/camera/1_10.fits | 2 +- hsc/camera/1_11.fits | 8 +- hsc/camera/1_12.fits | 4 +- hsc/camera/1_13.fits | 4 +- hsc/camera/1_14.fits | 4 +- hsc/camera/1_15.fits | 2 +- hsc/camera/1_16.fits | 6 +- hsc/camera/1_17.fits | 6 +- hsc/camera/1_18.fits | 2 +- hsc/camera/1_19.fits | 2 +- hsc/camera/1_20.fits | 2 +- hsc/camera/1_21.fits | 2 +- hsc/camera/1_22.fits | 2 +- hsc/camera/1_23.fits | 2 +- hsc/camera/1_24.fits | 2 +- hsc/camera/1_25.fits | 2 +- hsc/camera/1_26.fits | 2 +- hsc/camera/1_27.fits | 4 +- hsc/camera/1_28.fits | 4 +- hsc/camera/1_29.fits | 2 +- hsc/camera/1_30.fits | 6 +- hsc/camera/1_31.fits | 6 +- hsc/camera/1_32.fits | 2 +- hsc/camera/1_33.fits | 4 +- hsc/camera/1_34.fits | 2 +- hsc/camera/1_35.fits | 4 +- hsc/camera/1_36.fits | 4 +- hsc/camera/1_37.fits | 2 +- hsc/camera/1_38.fits | 4 +- hsc/camera/1_39.fits | 2 +- hsc/camera/1_40.fits | 2 +- hsc/camera/1_41.fits | 4 +- hsc/camera/1_42.fits | 2 +- hsc/camera/1_43.fits | 4 +- hsc/camera/1_44.fits | 4 +- hsc/camera/1_45.fits | 4 +- hsc/camera/1_46.fits | 2 +- hsc/camera/1_47.fits | 4 +- hsc/camera/1_48.fits | 4 +- hsc/camera/1_51.fits | 6 +- hsc/camera/1_52.fits | 4 +- hsc/camera/1_53.fits | 4 +- hsc/camera/1_54.fits | 4 +- hsc/camera/1_55.fits | 6 +- hsc/camera/1_56.fits | 6 +- hsc/camera/1_57.fits | 6 +- hsc/camera/camera.py | 940 ++++++++++++++++++++++++++++++------------------------------ include/lsst/obs/subaru/DistEstXYTransform.h | 62 ---- include/lsst/obs/subaru/HscDistortion.h | 91 ++++++ python/lsst/obs/hsc/transformRegistry.py | 271 ++++++++++++++++-- python/lsst/obs/subaru/subaruLib.i | 6 +- src/DistEstXYTransform.cc | 63 ---- src/HscDistortion.cc | 139 +++++++++ tests/hscDistortion.py | 119 ++++++++ ups/obs_subaru.cfg | 11 +- ups/obs_subaru.table | 1 - 123 files changed, 1294 insertions(+), 840 deletions(-)   commit b6aaa081fdf6b8522274ba94ab7cafe76cad6fa0 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:16:32 2015 -0400   generate new camera definitions for Suprime-Cam   suprimecam/camera/Chihiro.fits | 4 ++-- suprimecam/camera/Clarisse.fits | 6 ++--- suprimecam/camera/Fio.fits | 2 +- suprimecam/camera/Kiki.fits | 6 ++--- suprimecam/camera/Nausicaa.fits | 6 ++--- suprimecam/camera/Ponyo.fits | 4 ++-- suprimecam/camera/San.fits | 4 ++-- suprimecam/camera/Satsuki.fits | 2 +- suprimecam/camera/Sheeta.fits | 4 ++-- suprimecam/camera/Sophie.fits | 6 ++--- suprimecam/camera/camera.py | 88 ++++++++++++++++++++++++++++++++++------------------------------------ suprimecam/mit_camera/camera.py | 90 ++++++++++++++++++++++++++++++++++-------------------------------------- suprimecam/mit_camera/si001s.fits | 2 +- suprimecam/mit_camera/si002s.fits | 2 +- suprimecam/mit_camera/si005s.fits | 2 +- suprimecam/mit_camera/si006s.fits | 2 +- suprimecam/mit_camera/w4c5.fits | 2 +- suprimecam/mit_camera/w67c1.fits | 2 +- suprimecam/mit_camera/w6c1.fits | 2 +- suprimecam/mit_camera/w7c3.fits | 2 +- suprimecam/mit_camera/w93c2.fits | 2 +- suprimecam/mit_camera/w9c2.fits | 2 +- 22 files changed, 117 insertions(+), 125 deletions(-)   commit b9e6c60db15d415a3ed76196d32b2b35017c0242 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:16:58 2015 -0400   cleanup lib/Sconscript   lib/SConscript | 2 -- 1 file changed, 2 deletions(-)   commit eee651a591ca300ca5524f1c5543cbae1b9dd8c3 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:18:03 2015 -0400   add image of CCD positions for HSC This is useful for debugging. Taken from http://subarutelescope.org/Observing/Instruments/HSC/CCDPosition_20140811.png   hsc/CCDPosition_20140811.png | Bin 0 -> 370956 bytes 1 file changed, 0 insertions(+), 0 deletions(-)
            price Paul Price made changes -
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            swinbank John Swinbank added a comment -

            Thanks Paul – I'll look at this as soon as I get a moment. Should be within a couple of days.

            Show
            swinbank John Swinbank added a comment - Thanks Paul – I'll look at this as soon as I get a moment. Should be within a couple of days.
            Hide
            swinbank John Swinbank added a comment - - edited

            To start, I should apologise — this is my first time looking in any detail at an obs_ repository, and I suspect some things I’ll query are a matter of established convention rather than an innovation you’ve introduced on this ticket. I hope you’ll bear with me.

            A couple of very minor general comments:

            • Our guidelines suggest that summary line in commit messages should be capitalized.
            • I note you often include a filename in the summary (genDefectFits.py: fix instantiation of mapper, etc). I have no serious objection to this, but I do wonder how helpful it is given that the files may later be renamed, or even your patch may apply somewhere quite different if it ever gets cherry-picked.

            Now some per-commit queries:

            • 59f92b
              • By my reading, root=“.” is the default in the general CameraMapper class, but the obs_subaru mappers fall back to a SUPRIME_DATA_DIR environment variable instead. Except… here you’ve over-riding that fallback. I’m sure there are good reasons, but maybe a comment would be a appropriate to explain why?
            • a2780b
              • Looks like PgSqlConfig is currently only exists on HSC. Is that something we need to bring over to LSST?
            • 030278
              • I don’t understand what this does. I revert this commit, everything still builds fine when I run scons in the top level directory. Even with this commit included, it breaks if I just run scons hsc (ImportError: No module named subaruLib). So what’s this for?
            • 555767
              • Having defined PIXELSIZE at line 17, why not use that constant at line 83?
            • 111d2e:
              • Line 47 of HscDistortion.cc is (just!) too long.
              • The big chunk of pickle in tests/hscDistortion.py makes me a bit uncomfortable: it’s big and opaque and hard to diff (not, I suppose, that you expect it to change in future). Did you consider alternative representations? There are “only” 112 CCDs represented in total: simply directly constructing them in Python code doesn’t sound impossible.
              • The test case you’ve provided is nice, in that it demonstrates your package really does replicate the functionality in HSC’s distEst. But it’s really quite a high-level sort of verification test, rather than a direct unit test of the HscDistortion functionality. Would it be worth including some simple tests where you directly invoke HscDistortion and demonstrate the basic functionality?
              • I see that the numbers in transformRegistry.py have been transplanted from HSC’s distEst, which is fair enough. But I wonder if future generations might thank you for including a bit more information as to their provenance? I see the commit on HSC assures us they were generated from real data, but is that data accessible anywhere? How could I regenerate them if I thought they were faulty in some way? (Maybe these answers are already lost in the mists of time, but if so even noting that might be relevant).
              • There are a few other magic defaults in transformRegistry.pyccdToSkyOrder=9, tolerance=5e-3 etc. Why these values? Maybe a comment to explain?
              • This may be due to my misunderstanding of how things are supposed to work, but: if I run bin/genCameraRepository.py hsc/hsc_geom.paf hsc hsc_test and compare the results in hsc_test with those this commit adds to hsc/camera, they are not the same. Is that expected? If there’s some extra magic I need to do to generate the repository, it would be good to document somewhere.
              • It’s probably outside the scope of this review since you are updating rather than adding files, but: assuming I am correct and that the files in hsc/camera should be simply the output of bin/genCameraRepository.py, why are they being committed to the repository rather than generated at build time?
            • b6aaa0
              • I have the same concern here as above: if these files can be generated by the contents of the repository, what are we gaining by versioning them directly?

            I hope you'll consider the above (and maybe answer some of my questions!), but I don't feel the need to look a this again before it's merged. However, I'm leaving it in review in case Lauren MacArthur wants to add anything.

            Show
            swinbank John Swinbank added a comment - - edited To start, I should apologise — this is my first time looking in any detail at an obs_ repository, and I suspect some things I’ll query are a matter of established convention rather than an innovation you’ve introduced on this ticket. I hope you’ll bear with me. A couple of very minor general comments: Our guidelines suggest that summary line in commit messages should be capitalized. I note you often include a filename in the summary ( genDefectFits.py: fix instantiation of mapper , etc). I have no serious objection to this, but I do wonder how helpful it is given that the files may later be renamed, or even your patch may apply somewhere quite different if it ever gets cherry-picked. Now some per-commit queries: 59f92b By my reading, root=“.” is the default in the general CameraMapper class, but the obs_subaru mappers fall back to a SUPRIME_DATA_DIR environment variable instead. Except… here you’ve over-riding that fallback. I’m sure there are good reasons, but maybe a comment would be a appropriate to explain why? a2780b Looks like PgSqlConfig is currently only exists on HSC. Is that something we need to bring over to LSST? 030278 I don’t understand what this does. I revert this commit, everything still builds fine when I run scons in the top level directory. Even with this commit included, it breaks if I just run scons hsc ( ImportError: No module named subaruLib ). So what’s this for? 555767 Having defined PIXELSIZE at line 17, why not use that constant at line 83? 3065f1 Fine, no comments. 111d2e : Line 47 of HscDistortion.cc is (just!) too long. The big chunk of pickle in tests/hscDistortion.py makes me a bit uncomfortable: it’s big and opaque and hard to diff (not, I suppose, that you expect it to change in future). Did you consider alternative representations? There are “only” 112 CCDs represented in total: simply directly constructing them in Python code doesn’t sound impossible. The test case you’ve provided is nice, in that it demonstrates your package really does replicate the functionality in HSC’s distEst . But it’s really quite a high-level sort of verification test, rather than a direct unit test of the HscDistortion functionality. Would it be worth including some simple tests where you directly invoke HscDistortion and demonstrate the basic functionality? I see that the numbers in transformRegistry.py have been transplanted from HSC’s distEst , which is fair enough. But I wonder if future generations might thank you for including a bit more information as to their provenance? I see the commit on HSC assures us they were generated from real data, but is that data accessible anywhere? How could I regenerate them if I thought they were faulty in some way? (Maybe these answers are already lost in the mists of time, but if so even noting that might be relevant). There are a few other magic defaults in transformRegistry.py — ccdToSkyOrder=9 , tolerance=5e-3 etc. Why these values? Maybe a comment to explain? This may be due to my misunderstanding of how things are supposed to work, but: if I run bin/genCameraRepository.py hsc/hsc_geom.paf hsc hsc_test and compare the results in hsc_test with those this commit adds to hsc/camera , they are not the same. Is that expected? If there’s some extra magic I need to do to generate the repository, it would be good to document somewhere. It’s probably outside the scope of this review since you are updating rather than adding files, but: assuming I am correct and that the files in hsc/camera should be simply the output of bin/genCameraRepository.py , why are they being committed to the repository rather than generated at build time? b6aaa0 I have the same concern here as above: if these files can be generated by the contents of the repository, what are we gaining by versioning them directly? * b9e6c6 and eee651 Fine, no comments. I hope you'll consider the above (and maybe answer some of my questions!), but I don't feel the need to look a this again before it's merged. However, I'm leaving it in review in case Lauren MacArthur wants to add anything.
            Hide
            swinbank John Swinbank added a comment -

            On GitHub, Tim Jenness comments:

            Can we mark FITS files as binary files using gitattributes so Github doesn't try to show us the diff.

            If it really is necessary to version these files (see my comments above, but I'm certainly prepared to believe that it is), then I definitely second Tim's suggestion.

            Show
            swinbank John Swinbank added a comment - On GitHub, Tim Jenness comments: Can we mark FITS files as binary files using gitattributes so Github doesn't try to show us the diff. If it really is necessary to version these files (see my comments above, but I'm certainly prepared to believe that it is), then I definitely second Tim's suggestion.
            Hide
            tjenness Tim Jenness added a comment -

            I was thinking of suggesting that we declare FITS files to be binaries in all our repos. In theory we could get clever with a diff tool (by calling fitsheader) but then we'd have to ensure that that command (or the wcsLib version) was always available.

            Show
            tjenness Tim Jenness added a comment - I was thinking of suggesting that we declare FITS files to be binaries in all our repos. In theory we could get clever with a diff tool (by calling fitsheader ) but then we'd have to ensure that that command (or the wcsLib version) was always available.
            swinbank John Swinbank made changes -
            Sprint Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4, Science Pipelines DM-S15-5 [ 140, 151, 155, 159, 162 ] Science Pipelines DM-S15-1, Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4, Science Pipelines DM-S15-5, Science Pipelines DM-S15-6 [ 140, 151, 155, 159, 162, 165 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            Hide
            price Paul Price added a comment -
            • 59f92b: added explanation to commit message
            • a2780b: we added some code on the HSC side to support using a postgresql database for the registry. I don't know if we use it, but I think that given the coming changes to the butler, we should only worry about this when the time comes to pull things back to HSC.
            • 030278: added explanation to commit message
            • 555767: fixed
            • 111d2e:
              • Aha, I discovered an off-by-one bug in my reckoning of line lengths in emacs.
              • Changed the pickle to actual code.
              • I think the test does demonstrate the functionality as it's intended to be used. I don't think we're supposed to instantiate HscDistortion (or other distortion classes) explicitly.
              • Added some notes on the provenance of values in transformRegistry.py.
              • Added the files from re-running genCameraRepository.py. Suspect an old version got in there somehow.
              • I don't think we want to recreate the camera files (created by genCameraRepository.py) at build time, as this is done using "policy" files and an old package (pex_policy) that we expect will be completely removed soon.

            Cleaning up, will add something to mark FITS files as binary, and then hope to merge later this afternoon.

            Show
            price Paul Price added a comment - 59f92b: added explanation to commit message a2780b: we added some code on the HSC side to support using a postgresql database for the registry. I don't know if we use it, but I think that given the coming changes to the butler, we should only worry about this when the time comes to pull things back to HSC. 030278: added explanation to commit message 555767: fixed 111d2e: Aha, I discovered an off-by-one bug in my reckoning of line lengths in emacs. Changed the pickle to actual code. I think the test does demonstrate the functionality as it's intended to be used. I don't think we're supposed to instantiate HscDistortion (or other distortion classes) explicitly. Added some notes on the provenance of values in transformRegistry.py . Added the files from re-running genCameraRepository.py . Suspect an old version got in there somehow. I don't think we want to recreate the camera files (created by genCameraRepository.py) at build time, as this is done using "policy" files and an old package (pex_policy) that we expect will be completely removed soon. Cleaning up, will add something to mark FITS files as binary, and then hope to merge later this afternoon.
            Hide
            price Paul Price added a comment -

            Here's where we stand. I plan to merge this in about an hour.

            price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794=) $ git sub
            commit a59f6e69b6546cafa30d48e8f7b6778c48453a31
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:53:55 2015 -0400
             
                genDefectFits.py: fix instantiation of mapper
                
                We need a root directory, and SUPRIME_DATA_DIR may not be set.
                It doesn't really matter what root directory we use, because
                all we really need is the camera.
             
             bin/genDefectFits.py | 2 +-
             1 file changed, 1 insertion(+), 1 deletion(-)
             
            commit a9604db5103714b4621673ee19e7b8441c2b0f40
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:54:32 2015 -0400
             
                registryInfo.py: don't require postgresql
             
             bin/registryInfo.py | 3 ++-
             1 file changed, 2 insertions(+), 1 deletion(-)
             
            commit 141a34cf8d05404aff12626497aebd7f7775cc60
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 19:56:06 2015 -0400
             
                require SWIGed module before building defects registry
                
                This fixes race conditions in parallel builds.
             
             hsc/SConscript | 4 +++-
             1 file changed, 3 insertions(+), 1 deletion(-)
             
            commit 47038cc6caa09e721da545e04319c96f9913eb9b
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Tue Aug 4 15:19:25 2015 -0400
             
                Add .gitattributes to treat FITS files as binary
             
             .gitattributes | 1 +
             1 file changed, 1 insertion(+)
             
            commit 9dab8501770c9bb647f49c8dfb27c4d97d37be54
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:03:30 2015 -0400
             
                genCameraRepository.py: convert to a plate scale of 1
                
                LSST likes to use mm/pixels, but Subaru works in nominal pixels from the
                boresite, or a plate scale of 1.  This is true for both Suprime-Cam and
                HSC.
                
                This does not include updates to the camera definition files, just the
                conversion.
             
             bin/genCameraRepository.py | 6 ++----
             1 file changed, 2 insertions(+), 4 deletions(-)
             
            commit 7b9d29b059bbcf002e829cfba699898479d5a458
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:07:46 2015 -0400
             
                fix amp names
                
                Subaru amplifiers aren't indexed in two dimensions, so no need for an
                extra number that's always zero.
             
             bin/genCameraRepository.py    | 2 +-
             python/lsst/obs/subaru/isr.py | 5 +----
             2 files changed, 2 insertions(+), 5 deletions(-)
             
            commit 69fc0b11ae53e480c10147b12ace1bd915826529
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:15:35 2015 -0400
             
                add HSC distortion model rather than using external package distEst
                
                This implements distEst using the new cameraGeom framework, and activates
                the new distortion model.
             
             bin/genCameraRepository.py                   |    5 +-
             hsc/camera/0_00.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_01.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_02.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_03.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_04.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_05.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_06.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_07.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_08.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_09.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_10.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_11.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_12.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_13.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_14.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_15.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_16.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_17.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_18.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_19.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_20.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_21.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_22.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_23.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_24.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_25.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_26.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_27.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_28.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_29.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_30.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_31.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_32.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_33.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_34.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_35.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_36.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_37.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_38.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_39.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_40.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_41.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_42.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_43.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_44.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_45.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_46.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_47.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_48.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_51.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_52.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_53.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_54.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_55.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_56.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/0_57.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_00.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_01.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_02.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_03.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_04.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_05.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_06.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_07.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_08.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_09.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_10.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_11.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_12.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_13.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_14.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_15.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_16.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_17.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_18.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_19.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_20.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_21.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_22.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_23.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_24.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_25.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_26.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_27.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_28.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_29.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_30.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_31.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_32.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_33.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_34.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_35.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_36.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_37.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_38.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_39.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_40.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_41.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_42.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_43.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_44.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_45.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_46.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_47.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_48.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_51.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_52.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_53.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_54.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_55.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_56.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/1_57.fits                         |  Bin 17280 -> 17280 bytes
             hsc/camera/camera.py                         |  940 +++++++++++++++++++--------------------
             include/lsst/obs/subaru/DistEstXYTransform.h |   62 ---
             include/lsst/obs/subaru/HscDistortion.h      |   91 ++++
             python/lsst/obs/hsc/transformRegistry.py     |  281 +++++++++++-
             python/lsst/obs/subaru/subaruLib.i           |    6 +-
             src/DistEstXYTransform.cc                    |   63 ---
             src/HscDistortion.cc                         |  139 ++++++
             tests/hscDistortion.py                       | 1238 ++++++++++++++++++++++++++++++++++++++++++++++++++++
             ups/obs_subaru.cfg                           |   11 +-
             ups/obs_subaru.table                         |    1 -
             123 files changed, 2210 insertions(+), 627 deletions(-)
             
            commit 8c0352f2c1ea86857db37d400cee15a89b79d3ab
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:16:32 2015 -0400
             
                generate new camera definitions for Suprime-Cam
             
             suprimecam/camera/Chihiro.fits    | Bin 17280 -> 17280 bytes
             suprimecam/camera/Clarisse.fits   | Bin 17280 -> 17280 bytes
             suprimecam/camera/Fio.fits        | Bin 17280 -> 17280 bytes
             suprimecam/camera/Kiki.fits       | Bin 17280 -> 17280 bytes
             suprimecam/camera/Nausicaa.fits   | Bin 17280 -> 17280 bytes
             suprimecam/camera/Ponyo.fits      | Bin 17280 -> 17280 bytes
             suprimecam/camera/San.fits        | Bin 17280 -> 17280 bytes
             suprimecam/camera/Satsuki.fits    | Bin 17280 -> 17280 bytes
             suprimecam/camera/Sheeta.fits     | Bin 17280 -> 17280 bytes
             suprimecam/camera/Sophie.fits     | Bin 17280 -> 17280 bytes
             suprimecam/camera/camera.py       |  88 ++++++++++++++++++++++++++++++--------------------------------
             suprimecam/mit_camera/camera.py   |  90 +++++++++++++++++++++++++++++++---------------------------------
             suprimecam/mit_camera/si001s.fits | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/si002s.fits | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/si005s.fits | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/si006s.fits | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w4c5.fits   | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w67c1.fits  | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w6c1.fits   | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w7c3.fits   | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w93c2.fits  | Bin 17280 -> 17280 bytes
             suprimecam/mit_camera/w9c2.fits   | Bin 17280 -> 17280 bytes
             22 files changed, 85 insertions(+), 93 deletions(-)
             
            commit 7259c05d09790420c90071e763ec1bd0137d1c7f
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:16:58 2015 -0400
             
                cleanup lib/Sconscript
             
             lib/SConscript | 2 --
             1 file changed, 2 deletions(-)
             
            commit d973d1b8595e82e4e2770d327e50fe3b3fb81fb2
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Jul 23 20:18:03 2015 -0400
             
                add image of CCD positions for HSC
                
                This is useful for debugging.
                
                Taken from http://subarutelescope.org/Observing/Instruments/HSC/CCDPosition_20140811.png
             
             .gitattributes               |   1 +
             hsc/CCDPosition_20140811.png | Bin 0 -> 370956 bytes
             2 files changed, 1 insertion(+)
            

            Show
            price Paul Price added a comment - Here's where we stand. I plan to merge this in about an hour. price@price-laptop:~/LSST/obs/subaru (tickets/DM-1794=) $ git sub commit a59f6e69b6546cafa30d48e8f7b6778c48453a31 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:53:55 2015 -0400   genDefectFits.py: fix instantiation of mapper We need a root directory, and SUPRIME_DATA_DIR may not be set. It doesn't really matter what root directory we use, because all we really need is the camera.   bin/genDefectFits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)   commit a9604db5103714b4621673ee19e7b8441c2b0f40 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:54:32 2015 -0400   registryInfo.py: don't require postgresql   bin/registryInfo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)   commit 141a34cf8d05404aff12626497aebd7f7775cc60 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:56:06 2015 -0400   require SWIGed module before building defects registry This fixes race conditions in parallel builds.   hsc/SConscript | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)   commit 47038cc6caa09e721da545e04319c96f9913eb9b Author: Paul Price <price@astro.princeton.edu> Date: Tue Aug 4 15:19:25 2015 -0400   Add .gitattributes to treat FITS files as binary   .gitattributes | 1 + 1 file changed, 1 insertion(+)   commit 9dab8501770c9bb647f49c8dfb27c4d97d37be54 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:03:30 2015 -0400   genCameraRepository.py: convert to a plate scale of 1 LSST likes to use mm/pixels, but Subaru works in nominal pixels from the boresite, or a plate scale of 1. This is true for both Suprime-Cam and HSC. This does not include updates to the camera definition files, just the conversion.   bin/genCameraRepository.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)   commit 7b9d29b059bbcf002e829cfba699898479d5a458 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:07:46 2015 -0400   fix amp names Subaru amplifiers aren't indexed in two dimensions, so no need for an extra number that's always zero.   bin/genCameraRepository.py | 2 +- python/lsst/obs/subaru/isr.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-)   commit 69fc0b11ae53e480c10147b12ace1bd915826529 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:15:35 2015 -0400   add HSC distortion model rather than using external package distEst This implements distEst using the new cameraGeom framework, and activates the new distortion model.   bin/genCameraRepository.py | 5 +- hsc/camera/0_00.fits | Bin 17280 -> 17280 bytes hsc/camera/0_01.fits | Bin 17280 -> 17280 bytes hsc/camera/0_02.fits | Bin 17280 -> 17280 bytes hsc/camera/0_03.fits | Bin 17280 -> 17280 bytes hsc/camera/0_04.fits | Bin 17280 -> 17280 bytes hsc/camera/0_05.fits | Bin 17280 -> 17280 bytes hsc/camera/0_06.fits | Bin 17280 -> 17280 bytes hsc/camera/0_07.fits | Bin 17280 -> 17280 bytes hsc/camera/0_08.fits | Bin 17280 -> 17280 bytes hsc/camera/0_09.fits | Bin 17280 -> 17280 bytes hsc/camera/0_10.fits | Bin 17280 -> 17280 bytes hsc/camera/0_11.fits | Bin 17280 -> 17280 bytes hsc/camera/0_12.fits | Bin 17280 -> 17280 bytes hsc/camera/0_13.fits | Bin 17280 -> 17280 bytes hsc/camera/0_14.fits | Bin 17280 -> 17280 bytes hsc/camera/0_15.fits | Bin 17280 -> 17280 bytes hsc/camera/0_16.fits | Bin 17280 -> 17280 bytes hsc/camera/0_17.fits | Bin 17280 -> 17280 bytes hsc/camera/0_18.fits | Bin 17280 -> 17280 bytes hsc/camera/0_19.fits | Bin 17280 -> 17280 bytes hsc/camera/0_20.fits | Bin 17280 -> 17280 bytes hsc/camera/0_21.fits | Bin 17280 -> 17280 bytes hsc/camera/0_22.fits | Bin 17280 -> 17280 bytes hsc/camera/0_23.fits | Bin 17280 -> 17280 bytes hsc/camera/0_24.fits | Bin 17280 -> 17280 bytes hsc/camera/0_25.fits | Bin 17280 -> 17280 bytes hsc/camera/0_26.fits | Bin 17280 -> 17280 bytes hsc/camera/0_27.fits | Bin 17280 -> 17280 bytes hsc/camera/0_28.fits | Bin 17280 -> 17280 bytes hsc/camera/0_29.fits | Bin 17280 -> 17280 bytes hsc/camera/0_30.fits | Bin 17280 -> 17280 bytes hsc/camera/0_31.fits | Bin 17280 -> 17280 bytes hsc/camera/0_32.fits | Bin 17280 -> 17280 bytes hsc/camera/0_33.fits | Bin 17280 -> 17280 bytes hsc/camera/0_34.fits | Bin 17280 -> 17280 bytes hsc/camera/0_35.fits | Bin 17280 -> 17280 bytes hsc/camera/0_36.fits | Bin 17280 -> 17280 bytes hsc/camera/0_37.fits | Bin 17280 -> 17280 bytes hsc/camera/0_38.fits | Bin 17280 -> 17280 bytes hsc/camera/0_39.fits | Bin 17280 -> 17280 bytes hsc/camera/0_40.fits | Bin 17280 -> 17280 bytes hsc/camera/0_41.fits | Bin 17280 -> 17280 bytes hsc/camera/0_42.fits | Bin 17280 -> 17280 bytes hsc/camera/0_43.fits | Bin 17280 -> 17280 bytes hsc/camera/0_44.fits | Bin 17280 -> 17280 bytes hsc/camera/0_45.fits | Bin 17280 -> 17280 bytes hsc/camera/0_46.fits | Bin 17280 -> 17280 bytes hsc/camera/0_47.fits | Bin 17280 -> 17280 bytes hsc/camera/0_48.fits | Bin 17280 -> 17280 bytes hsc/camera/0_51.fits | Bin 17280 -> 17280 bytes hsc/camera/0_52.fits | Bin 17280 -> 17280 bytes hsc/camera/0_53.fits | Bin 17280 -> 17280 bytes hsc/camera/0_54.fits | Bin 17280 -> 17280 bytes hsc/camera/0_55.fits | Bin 17280 -> 17280 bytes hsc/camera/0_56.fits | Bin 17280 -> 17280 bytes hsc/camera/0_57.fits | Bin 17280 -> 17280 bytes hsc/camera/1_00.fits | Bin 17280 -> 17280 bytes hsc/camera/1_01.fits | Bin 17280 -> 17280 bytes hsc/camera/1_02.fits | Bin 17280 -> 17280 bytes hsc/camera/1_03.fits | Bin 17280 -> 17280 bytes hsc/camera/1_04.fits | Bin 17280 -> 17280 bytes hsc/camera/1_05.fits | Bin 17280 -> 17280 bytes hsc/camera/1_06.fits | Bin 17280 -> 17280 bytes hsc/camera/1_07.fits | Bin 17280 -> 17280 bytes hsc/camera/1_08.fits | Bin 17280 -> 17280 bytes hsc/camera/1_09.fits | Bin 17280 -> 17280 bytes hsc/camera/1_10.fits | Bin 17280 -> 17280 bytes hsc/camera/1_11.fits | Bin 17280 -> 17280 bytes hsc/camera/1_12.fits | Bin 17280 -> 17280 bytes hsc/camera/1_13.fits | Bin 17280 -> 17280 bytes hsc/camera/1_14.fits | Bin 17280 -> 17280 bytes hsc/camera/1_15.fits | Bin 17280 -> 17280 bytes hsc/camera/1_16.fits | Bin 17280 -> 17280 bytes hsc/camera/1_17.fits | Bin 17280 -> 17280 bytes hsc/camera/1_18.fits | Bin 17280 -> 17280 bytes hsc/camera/1_19.fits | Bin 17280 -> 17280 bytes hsc/camera/1_20.fits | Bin 17280 -> 17280 bytes hsc/camera/1_21.fits | Bin 17280 -> 17280 bytes hsc/camera/1_22.fits | Bin 17280 -> 17280 bytes hsc/camera/1_23.fits | Bin 17280 -> 17280 bytes hsc/camera/1_24.fits | Bin 17280 -> 17280 bytes hsc/camera/1_25.fits | Bin 17280 -> 17280 bytes hsc/camera/1_26.fits | Bin 17280 -> 17280 bytes hsc/camera/1_27.fits | Bin 17280 -> 17280 bytes hsc/camera/1_28.fits | Bin 17280 -> 17280 bytes hsc/camera/1_29.fits | Bin 17280 -> 17280 bytes hsc/camera/1_30.fits | Bin 17280 -> 17280 bytes hsc/camera/1_31.fits | Bin 17280 -> 17280 bytes hsc/camera/1_32.fits | Bin 17280 -> 17280 bytes hsc/camera/1_33.fits | Bin 17280 -> 17280 bytes hsc/camera/1_34.fits | Bin 17280 -> 17280 bytes hsc/camera/1_35.fits | Bin 17280 -> 17280 bytes hsc/camera/1_36.fits | Bin 17280 -> 17280 bytes hsc/camera/1_37.fits | Bin 17280 -> 17280 bytes hsc/camera/1_38.fits | Bin 17280 -> 17280 bytes hsc/camera/1_39.fits | Bin 17280 -> 17280 bytes hsc/camera/1_40.fits | Bin 17280 -> 17280 bytes hsc/camera/1_41.fits | Bin 17280 -> 17280 bytes hsc/camera/1_42.fits | Bin 17280 -> 17280 bytes hsc/camera/1_43.fits | Bin 17280 -> 17280 bytes hsc/camera/1_44.fits | Bin 17280 -> 17280 bytes hsc/camera/1_45.fits | Bin 17280 -> 17280 bytes hsc/camera/1_46.fits | Bin 17280 -> 17280 bytes hsc/camera/1_47.fits | Bin 17280 -> 17280 bytes hsc/camera/1_48.fits | Bin 17280 -> 17280 bytes hsc/camera/1_51.fits | Bin 17280 -> 17280 bytes hsc/camera/1_52.fits | Bin 17280 -> 17280 bytes hsc/camera/1_53.fits | Bin 17280 -> 17280 bytes hsc/camera/1_54.fits | Bin 17280 -> 17280 bytes hsc/camera/1_55.fits | Bin 17280 -> 17280 bytes hsc/camera/1_56.fits | Bin 17280 -> 17280 bytes hsc/camera/1_57.fits | Bin 17280 -> 17280 bytes hsc/camera/camera.py | 940 +++++++++++++++++++-------------------- include/lsst/obs/subaru/DistEstXYTransform.h | 62 --- include/lsst/obs/subaru/HscDistortion.h | 91 ++++ python/lsst/obs/hsc/transformRegistry.py | 281 +++++++++++- python/lsst/obs/subaru/subaruLib.i | 6 +- src/DistEstXYTransform.cc | 63 --- src/HscDistortion.cc | 139 ++++++ tests/hscDistortion.py | 1238 ++++++++++++++++++++++++++++++++++++++++++++++++++++ ups/obs_subaru.cfg | 11 +- ups/obs_subaru.table | 1 - 123 files changed, 2210 insertions(+), 627 deletions(-)   commit 8c0352f2c1ea86857db37d400cee15a89b79d3ab Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:16:32 2015 -0400   generate new camera definitions for Suprime-Cam   suprimecam/camera/Chihiro.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Clarisse.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Fio.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Kiki.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Nausicaa.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Ponyo.fits | Bin 17280 -> 17280 bytes suprimecam/camera/San.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Satsuki.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Sheeta.fits | Bin 17280 -> 17280 bytes suprimecam/camera/Sophie.fits | Bin 17280 -> 17280 bytes suprimecam/camera/camera.py | 88 ++++++++++++++++++++++++++++++-------------------------------- suprimecam/mit_camera/camera.py | 90 +++++++++++++++++++++++++++++++--------------------------------- suprimecam/mit_camera/si001s.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/si002s.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/si005s.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/si006s.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w4c5.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w67c1.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w6c1.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w7c3.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w93c2.fits | Bin 17280 -> 17280 bytes suprimecam/mit_camera/w9c2.fits | Bin 17280 -> 17280 bytes 22 files changed, 85 insertions(+), 93 deletions(-)   commit 7259c05d09790420c90071e763ec1bd0137d1c7f Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:16:58 2015 -0400   cleanup lib/Sconscript   lib/SConscript | 2 -- 1 file changed, 2 deletions(-)   commit d973d1b8595e82e4e2770d327e50fe3b3fb81fb2 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 20:18:03 2015 -0400   add image of CCD positions for HSC This is useful for debugging. Taken from http://subarutelescope.org/Observing/Instruments/HSC/CCDPosition_20140811.png   .gitattributes | 1 + hsc/CCDPosition_20140811.png | Bin 0 -> 370956 bytes 2 files changed, 1 insertion(+)
            price Paul Price made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            swinbank John Swinbank added a comment -

            Thanks for the clarifications and explanations!

            Show
            swinbank John Swinbank added a comment - Thanks for the clarifications and explanations!
            Hide
            price Paul Price added a comment -

            Merged to master. Hooray!

            Show
            price Paul Price added a comment - Merged to master. Hooray!
            price Paul Price made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            swinbank John Swinbank made changes -
            Labels HSC distortion legacy distortion legacy

              People

              Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              John Swinbank, Lauren MacArthur
              Watchers:
              John Swinbank, Kian-Tat Lim, Lauren MacArthur, Paul Price, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.