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

Replace all uses of Calib with PhotoCalib

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Story Points:
      10
    • Sprint:
      AP S19-2, AP S19-3, AP S19-4, AP S19-5
    • Team:
      Alert Production

      Description

      Search and replace all "Calib" with "PhotoCalib", delete the Calib tests from afw/tests/testColors.py, delete all the Calib code (include/Calib.h, src/Calib.cc, python/calib.cc), and then see what breaks. The API is now quite different and it's immutable, so there are going to be quite a few more changes necessary. Hopefully most of them can be taken care of by globally replacing "getMagnitude" with "countsToMagnitude", but we'll see.

        Attachments

          Issue Links

            Activity

            Parejkoj John Parejko created issue -
            Parejkoj John Parejko made changes -
            Field Original Value New Value
            Epic Link DM-10153 [ 31778 ]
            Parejkoj John Parejko made changes -
            Story Points 10
            Assignee John Parejko [ parejkoj ]
            Parejkoj John Parejko made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Parejkoj John Parejko made changes -
            Comment [ I wish we could have replaced Calib with PhotoCalib for this, but I think I can manage it (with a bunch of internal conversions). ]
            Parejkoj John Parejko made changes -
            Status In Progress [ 3 ] To Do [ 10001 ]
            Parejkoj John Parejko made changes -
            Link This issue is blocked by DM-16903 [ DM-16903 ]
            Parejkoj John Parejko made changes -
            Link This issue is blocked by DM-10346 [ DM-10346 ]
            Parejkoj John Parejko made changes -
            Link This issue contains DM-15544 [ DM-15544 ]
            swinbank John Swinbank made changes -
            Link This issue is blocked by DM-16926 [ DM-16926 ]
            swinbank John Swinbank made changes -
            Sprint AP S19-2 [ 830 ]
            swinbank John Swinbank made changes -
            Sprint AP S19-2 [ 830 ] AP S19-2, AP S19-3 [ 830, 831 ]
            Parejkoj John Parejko made changes -
            Link This issue relates to DM-17873 [ DM-17873 ]
            Parejkoj John Parejko made changes -
            Link This issue is blocked by DM-18067 [ DM-18067 ]
            Parejkoj John Parejko made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            swinbank John Swinbank made changes -
            Sprint AP S19-2, AP S19-3 [ 830, 831 ] AP S19-2, AP S19-3, AP S19-4 [ 830, 831, 832 ]
            Parejkoj John Parejko made changes -
            Link This issue is triggering DM-18157 [ DM-18157 ]
            Parejkoj John Parejko made changes -
            Link This issue relates to DM-18492 [ DM-18492 ]
            Hide
            Parejkoj John Parejko added a comment - - edited
            Show
            Parejkoj John Parejko added a comment - - edited We're in the home stretch! Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29541/pipeline
            Parejkoj John Parejko made changes -
            Link This issue is triggering DM-18544 [ DM-18544 ]
            Hide
            Parejkoj John Parejko added a comment - - edited

            This is going to be fun! If at all possible, I would like to have the reviews before the end of this week, so that I'll have a chance of merging next week: chasing rebasing is already getting tricky. If any of you do not believe you'll be able to do that, please let me know ASAP so I can find another reviewer for your part.

            There are 20 PRs associated with this ticket, and some of them only need reviews of one or two commits on the PR (because they're built on DM-17029). I'm providing direct links to either the PR, or the commits each person is to review. Please leave at least a Comment on github, in addition to removing your name from the reviewer list here per the dev guide. I tried to keep all of the commits atomic.

            Christopher Waters: afw

            Nate Lust: daf_butler validate_drp

            Simon Krughoff: obs_base obs_test obs_decam obs_lsstSim obs_sdss obs_monocam

            Chris Morrison: meas_base meas_alogrithms ap_association

            Meredith Rawls: ip_isr meas_astrom

            Krzysztof Findeisen: meas_modelfit lsst_dm_stack_demo

            Ian Sullivan: ip_diffim jointcal

            Yusra AlSayyad: pipe_tasks meas_mosaic

            Show
            Parejkoj John Parejko added a comment - - edited This is going to be fun! If at all possible, I would like to have the reviews before the end of this week, so that I'll have a chance of merging next week: chasing rebasing is already getting tricky. If any of you do not believe you'll be able to do that, please let me know ASAP so I can find another reviewer for your part. There are 20 PRs associated with this ticket, and some of them only need reviews of one or two commits on the PR (because they're built on DM-17029 ). I'm providing direct links to either the PR, or the commits each person is to review. Please leave at least a Comment on github, in addition to removing your name from the reviewer list here per the dev guide . I tried to keep all of the commits atomic. Christopher Waters : afw https://github.com/lsst/afw/pull/442 (Chris: you get the big one here: let me know right away if I need to find another reviewer, or if I should split it up) Nate Lust : daf_butler validate_drp https://github.com/lsst/daf_butler/pull/139 https://github.com/lsst/validate_drp/pull/102 Simon Krughoff : obs_base obs_test obs_decam obs_lsstSim obs_sdss obs_monocam https://github.com/lsst/obs_base/pull/140 https://github.com/lsst/obs_test/pull/63 https://github.com/lsst/obs_decam/pull/105 https://github.com/lsst/obs_lsstSim/pull/83 https://github.com/lsst/obs_sdss/pull/60 https://github.com/lsst/obs_monocam/pull/16 Chris Morrison : meas_base meas_alogrithms ap_association https://github.com/lsst/meas_base/pull/141 https://github.com/lsst/meas_algorithms/pull/156/commits/c890a65563d16497f99d3d7536c934082a6b2545 https://github.com/lsst/meas_algorithms/pull/156/commits/c4412c2b7c6b3e731ba83a04d9a64b3d6d10a488 https://github.com/lsst/ap_association/pull/44 Meredith Rawls : ip_isr meas_astrom https://github.com/lsst/ip_isr/pull/82 https://github.com/lsst/meas_astrom/pull/118 https://github.com/lsst/meas_extensions_astrometryNet/pull/21 Krzysztof Findeisen : meas_modelfit lsst_dm_stack_demo https://github.com/lsst/meas_modelfit/pull/79 https://github.com/lsst/lsst_dm_stack_demo/pull/28 Ian Sullivan : ip_diffim jointcal https://github.com/lsst/ip_diffim/pull/116 https://github.com/lsst/jointcal/pull/132/commits/4c9f13e5f2ff58c28438c5c2d0c137e967568469 Yusra AlSayyad : pipe_tasks meas_mosaic https://github.com/lsst/pipe_tasks/pull/282/commits/1a77c19a4f024dfc25d2d8bf7f5c1fca64242a52 https://github.com/lsst/meas_mosaic/pull/49 Is there some kind of HSC test run you would like to do as a larger integration test? ci_hsc passed on my machine.
            Parejkoj John Parejko made changes -
            Reviewers Chris Morrison, Christopher Waters, Ian Sullivan, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cmorrison, cwaters, sullivan, krzys, mrawls, nlust, krughoff, yusra ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            cmorrison Chris Morrison added a comment -

            The meas_algorithm links for me appear to be broken. Also there are no links to meas_astrom but you mention it in my chunk of code to review?

            Show
            cmorrison Chris Morrison added a comment - The meas_algorithm links for me appear to be broken. Also there are no links to meas_astrom but you mention it in my chunk of code to review?
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for checking that; the rebase strikes again. I've fixed the meas_algorithms links. Meredith got meas_astrom, but I accidentally noted it in your header instead of hers.

            Show
            Parejkoj John Parejko added a comment - Thanks for checking that; the rebase strikes again. I've fixed the meas_algorithms links. Meredith got meas_astrom, but I accidentally noted it in your header instead of hers.
            sullivan Ian Sullivan made changes -
            Reviewers Chris Morrison, Christopher Waters, Ian Sullivan, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cmorrison, cwaters, sullivan, krzys, mrawls, nlust, krughoff, yusra ] Chris Morrison, Christopher Waters, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cmorrison, cwaters, krzys, mrawls, nlust, krughoff, yusra ]
            Hide
            sullivan Ian Sullivan added a comment -

            I have left reviews for both ip_diffim and jointcal on GitHub. Both are fine to merge.

            Show
            sullivan Ian Sullivan added a comment - I have left reviews for both ip_diffim and jointcal on GitHub. Both are fine to merge.
            Hide
            cmorrison Chris Morrison added a comment -

            Assuming it all passes Jenkins, I'm cool with clearing the my part of the ticket.

            Show
            cmorrison Chris Morrison added a comment - Assuming it all passes Jenkins, I'm cool with clearing the my part of the ticket.
            cmorrison Chris Morrison made changes -
            Reviewers Chris Morrison, Christopher Waters, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cmorrison, cwaters, krzys, mrawls, nlust, krughoff, yusra ] Christopher Waters, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, krzys, mrawls, nlust, krughoff, yusra ]
            krzys Krzysztof Findeisen made changes -
            Reviewers Christopher Waters, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, krzys, mrawls, nlust, krughoff, yusra ] Christopher Waters, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, mrawls, nlust, krughoff, yusra ]
            Hide
            nlust Nate Lust added a comment -

            I am going to be running a gen3 demo against the stack with this ticket branch on it tonight/in the morning to both check the daf_butler stuff and all the changes against gen3. I'll let you know results asap.

            Show
            nlust Nate Lust added a comment - I am going to be running a gen3 demo against the stack with this ticket branch on it tonight/in the morning to both check the daf_butler stuff and all the changes against gen3. I'll let you know results asap.
            Hide
            czw Christopher Waters added a comment -

            I had a few minor comments, but I don't have any big concerns with afw.

            Show
            czw Christopher Waters added a comment - I had a few minor comments, but I don't have any big concerns with afw.
            mrawls Meredith Rawls made changes -
            Reviewers Christopher Waters, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, mrawls, nlust, krughoff, yusra ] Christopher Waters, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, nlust, krughoff, yusra ]
            czw Christopher Waters made changes -
            Reviewers Christopher Waters, Nate Lust, Simon Krughoff, Yusra AlSayyad [ cwaters, nlust, krughoff, yusra ] Nate Lust, Simon Krughoff, Yusra AlSayyad [ nlust, krughoff, yusra ]
            nlust Nate Lust made changes -
            Watchers Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad ] Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Simon Krughoff, Yusra AlSayyad [ Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Simon Krughoff, Yusra AlSayyad ]
            Hide
            nlust Nate Lust added a comment -

            I am happy with how this looks after digging through some gen3 consequences. If you would like help running a final test against gen3 once all changes people give you are added and rebasing is done, I can help out with that.

            Show
            nlust Nate Lust added a comment - I am happy with how this looks after digging through some gen3 consequences. If you would like help running a final test against gen3 once all changes people give you are added and rebasing is done, I can help out with that.
            Hide
            krughoff Simon Krughoff added a comment -

            The obs packages look fine to me.

            Show
            krughoff Simon Krughoff added a comment - The obs packages look fine to me.
            krughoff Simon Krughoff made changes -
            Reviewers Nate Lust, Simon Krughoff, Yusra AlSayyad [ nlust, krughoff, yusra ] Nate Lust, Yusra AlSayyad [ nlust, yusra ]
            Hide
            yusra Yusra AlSayyad added a comment - - edited

            meas_mosaic fails.  See logs in /datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156/logs/ The traceback isn't useful as it's just telling you that it thinks the pixel scale is 0, which means that the fit failed.  

            I linked the diagnostics here: https://lsst-web.ncsa.illinois.edu/~yusra/DM-10156/  it only got as far as the coeffs.dat and ccd.dat which --> 0. 

            Also,   /datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156/logs/mosaic-9615-HSC-I-151571.log is  11626 lines long, the first ~11000 of which are this repeated over and over:

            [yusra@lsst-dev01 logs]$ tail -1000 mosaic-9615-HSC-I-151571.log  |head
            LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy.
            LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details.
            LoadIndexedReferenceObjectsTask INFO: Converted refcat flux fields to nJy (name, units): (g_flux, ''); (r_flux, ''); (i_flux, ''); (z_flux, ''); (y_flux, ''); (i_fluxSigma, ''); (y_fluxSigma, ''); (r_fluxSigma, ''); (z_fluxSigma, ''); (g_fluxSigma, '')
            LoadIndexedReferenceObjectsTask WARN: Found version 0 reference catalog with old style units in schema.
            LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy.
            LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details.
            

            Compare the first couple iterations

            solveMosaic_CCD: Before fitting calcChi2: 1.123693e+05 1.229126e+05
            solveMosaic_CCD: Before fitting matched: 2517.970 (arcsec) sources: 2674.897 (arcsec)
            nstar: 4936
            size : 12990
            Allocated   1.3 GB memory
            Number good: 229695, 18906
            solveMatrix_Eigen: FullPivLU Relative error = 0.000829995
            solveMosaic_CCD: 1th iteration calcChi2: 1.718002e+26 1.766372e+26
            solveMosaic_CCD: 1th iteration matched: 98455173921704.266 (arcsec) sources: 57293993512636.641 (arcsec)
            nreject = 1378
            nreject = 187
            nstar: 4912
            size : 12942
            Allocated   1.2 GB memory
            Number good: 228317, 18695
            solveMatrix_Eigen: FullPivLU Relative error = 6.03535e-05
            solveMosaic_CCD: 2th iteration calcChi2: 3.968653e+24 4.154747e+24
            solveMosaic_CCD: 2th iteration matched: 15009109434367.627 (arcsec) sources: 11293368742399.199 (arcsec)
            nreject = 2329
            nreject = 286
            

            with Hsin-Fang Chiang's w_2019_10: /datasets/hsc/repo/rerun/RC/w_2019_10/DM-17940/logs/mosaic/mosaic-9615-HSC-I-150563.log:

            solveMosaic_CCD: Before fitting calcChi2: 9.639419e-05 9.856314e-05
            solveMosaic_CCD: Before fitting matched: 0.074 (arcsec) sources: 0.038 (arcsec)
            nstar: 4936
            size : 12990
            Allocated   1.3 GB memory
            Number good: 229474, 18906
            solveMosaic_CCD: 1th iteration calcChi2: 8.147253e-05 8.165830e-05
            solveMosaic_CCD: 1th iteration matched: 0.068 (arcsec) sources: 0.011 (arcsec)
            nreject = 12
            nreject = 184
            nstar: 4915
            size : 12948
            Allocated   1.2 GB memory
            Number good: 229462, 18788
            solveMosaic_CCD: 2th iteration calcChi2: 8.135584e-05 8.144264e-05
            solveMosaic_CCD: 2th iteration matched: 0.068 (arcsec) sources: 0.008 (arcsec)
            nreject = 0
            nreject = 172
            nstar: 4895
            size : 12908
            Allocated   1.2 GB memory
            Number good: 229462, 18633
            

            Ideas?

            You can reproduce with:

             
            .  /project/yusra/lsstsw/bin/setup.sh
            setup lsst_distrib
             
            mosaic.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun private/<yourUsername>/DM-10156  --numCoresForReadSource=12 --id ccd=0..8^10..103 visit=1258^1262^1270^1274^1278^1280^1282^1286^1288^1290^1294^1300^1302^1306^1308^1310^1314^1316^1324^1326^1330^24494^24504^24522^24536^24538 tract=9615 --diagnostics --diagDir=/datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156/mosaic_diag/HSC-I --no-versions
            

            Show
            yusra Yusra AlSayyad added a comment - - edited meas_mosaic fails.  See logs in /datasets/hsc/repo/rerun/private/yusra/RC2/ DM-10156 /logs/ The traceback isn't useful as it's just telling you that it thinks the pixel scale is 0, which means that the fit failed.   I linked the diagnostics here:  https://lsst-web.ncsa.illinois.edu/~yusra/DM-10156/   it only got as far as the coeffs.dat and ccd.dat which --> 0.  Also,    /datasets/hsc/repo/rerun/private/yusra/RC2/ DM-10156 /logs/mosaic-9615-HSC-I-151571.log is  11626 lines long, the first ~11000 of which are this repeated over and over: [yusra@lsst-dev01 logs]$ tail -1000 mosaic-9615-HSC-I-151571.log |head LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy. LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details. LoadIndexedReferenceObjectsTask INFO: Converted refcat flux fields to nJy (name, units): (g_flux, ''); (r_flux, ''); (i_flux, ''); (z_flux, ''); (y_flux, ''); (i_fluxSigma, ''); (y_fluxSigma, ''); (r_fluxSigma, ''); (z_fluxSigma, ''); (g_fluxSigma, '') LoadIndexedReferenceObjectsTask WARN: Found version 0 reference catalog with old style units in schema. LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy. LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details. Compare the first couple iterations solveMosaic_CCD: Before fitting calcChi2: 1.123693e+05 1.229126e+05 solveMosaic_CCD: Before fitting matched: 2517.970 (arcsec) sources: 2674.897 (arcsec) nstar: 4936 size : 12990 Allocated 1.3 GB memory Number good: 229695, 18906 solveMatrix_Eigen: FullPivLU Relative error = 0.000829995 solveMosaic_CCD: 1th iteration calcChi2: 1.718002e+26 1.766372e+26 solveMosaic_CCD: 1th iteration matched: 98455173921704.266 (arcsec) sources: 57293993512636.641 (arcsec) nreject = 1378 nreject = 187 nstar: 4912 size : 12942 Allocated 1.2 GB memory Number good: 228317, 18695 solveMatrix_Eigen: FullPivLU Relative error = 6.03535e-05 solveMosaic_CCD: 2th iteration calcChi2: 3.968653e+24 4.154747e+24 solveMosaic_CCD: 2th iteration matched: 15009109434367.627 (arcsec) sources: 11293368742399.199 (arcsec) nreject = 2329 nreject = 286 with Hsin-Fang Chiang 's w_2019_10: /datasets/hsc/repo/rerun/RC/w_2019_10/ DM-17940 /logs/mosaic/mosaic-9615-HSC-I-150563.log : solveMosaic_CCD: Before fitting calcChi2: 9.639419e-05 9.856314e-05 solveMosaic_CCD: Before fitting matched: 0.074 (arcsec) sources: 0.038 (arcsec) nstar: 4936 size : 12990 Allocated 1.3 GB memory Number good: 229474, 18906 solveMosaic_CCD: 1th iteration calcChi2: 8.147253e-05 8.165830e-05 solveMosaic_CCD: 1th iteration matched: 0.068 (arcsec) sources: 0.011 (arcsec) nreject = 12 nreject = 184 nstar: 4915 size : 12948 Allocated 1.2 GB memory Number good: 229462, 18788 solveMosaic_CCD: 2th iteration calcChi2: 8.135584e-05 8.144264e-05 solveMosaic_CCD: 2th iteration matched: 0.068 (arcsec) sources: 0.008 (arcsec) nreject = 0 nreject = 172 nstar: 4895 size : 12908 Allocated 1.2 GB memory Number good: 229462, 18633 Ideas? You can reproduce with:   . /project/yusra/lsstsw/bin/setup.sh setup lsst_distrib   mosaic.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun private/<yourUsername>/DM-10156 --numCoresForReadSource=12 --id ccd=0..8^10..103 visit=1258^1262^1270^1274^1278^1280^1282^1286^1288^1290^1294^1300^1302^1306^1308^1310^1314^1316^1324^1326^1330^24494^24504^24522^24536^24538 tract=9615 --diagnostics --diagDir=/datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156/mosaic_diag/HSC-I --no-versions
            Hide
            Parejkoj John Parejko added a comment -

            Can you try running meas_mosaic with just DM-17029, to see whether the problem is due to the nJy reference catalog conversion? The conversion of meas_mosaic to use PhotoCalib was pretty straightforward (I don't know if you've reviewed that code yet).

            Show
            Parejkoj John Parejko added a comment - Can you try running meas_mosaic with just DM-17029 , to see whether the problem is due to the nJy reference catalog conversion? The conversion of meas_mosaic to use PhotoCalib was pretty straightforward (I don't know if you've reviewed that code yet).
            nlust Nate Lust made changes -
            Watchers Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad [ Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Nate Lust, Simon Krughoff, Yusra AlSayyad ] Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Simon Krughoff, Yusra AlSayyad [ Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Simon Krughoff, Yusra AlSayyad ]
            Hide
            yusra Yusra AlSayyad added a comment -

            Ran the above mosaic command with w_2019_12 +

            yusra@lsst-dev01 logs]$ eups list -s | grep LOCAL
            meas_algorithms       LOCAL:/home/yusra/lsst_devel/LSST/DMS/DM-17029/meas_algorithms    setup
            meas_extensions_astrometryNet LOCAL:/home/yusra/lsst_devel/LSST/DMS/DM-17029/meas_extensions_astrometryNet      setup
            

            It completed. See /datasets/hsc/repo/rerun/private/yusra/RC2/DM-17029/logs/mosaic_I.log.

            Sophie Reed ran coaddDriver and multiBand on jointcal-calibrated coadds, which means that all the pieces of a normal DRP work aside from meas_mosaic. She's using those outputs to test pipe_analysis with your deprecated interface ticket now, and to see if there are any surprise science effects.

            Show
            yusra Yusra AlSayyad added a comment - Ran the above mosaic command with w_2019_12 + yusra@lsst-dev01 logs]$ eups list -s | grep LOCAL meas_algorithms LOCAL:/home/yusra/lsst_devel/LSST/DMS/DM-17029/meas_algorithms setup meas_extensions_astrometryNet LOCAL:/home/yusra/lsst_devel/LSST/DMS/DM-17029/meas_extensions_astrometryNet setup It completed. See /datasets/hsc/repo/rerun/private/yusra/RC2/ DM-17029 /logs/mosaic_I.log . Sophie Reed ran coaddDriver and multiBand on jointcal-calibrated coadds, which means that all the pieces of a normal DRP work aside from meas_mosaic. She's using those outputs to test pipe_analysis with your deprecated interface ticket now, and to see if there are any surprise science effects.
            Hide
            Parejkoj John Parejko added a comment -

            I'm confused about how there can be thousands of the "Converted refcat flux fields to nJy" messages: that line is outside of any loops in loadSkyCircle, so it should only appear once when that is called. My first guess was that it was due to how MosaicTask handles multiprocessing, but that should have only caused ncpus worth of extra output (if that).

            Show
            Parejkoj John Parejko added a comment - I'm confused about how there can be thousands of the "Converted refcat flux fields to nJy" messages: that line is outside of any loops in loadSkyCircle , so it should only appear once when that is called. My first guess was that it was due to how MosaicTask handles multiprocessing, but that should have only caused ncpus worth of extra output (if that).
            Hide
            Parejkoj John Parejko added a comment -

            Scratch the above: it's definitely due to the use of multiprocessing. In MosaicTask, readCatalog calls pool.map_async(worker, params), where params are the dataRefs, and the worker does readSrc for each, which calls refObjLoader.joinMatchListWithCatalog, which does loadSkyCircle. So there's a loadSkyCircle call for every dataRef, and I'll bet that there are 2678 (the number of times that message appears) ccd+visits in that tract.

            Show
            Parejkoj John Parejko added a comment - Scratch the above: it's definitely due to the use of multiprocessing. In MosaicTask, readCatalog calls pool.map_async(worker, params) , where params are the dataRefs, and the worker does readSrc for each, which calls refObjLoader.joinMatchListWithCatalog , which does loadSkyCircle . So there's a loadSkyCircle call for every dataRef, and I'll bet that there are 2678 (the number of times that message appears) ccd+visits in that tract.
            Hide
            Parejkoj John Parejko added a comment -

            Ah... I wonder if the problem is in mosaicTask.py:395: the code computes magnitudes from the refcat via -2.5*numpy.log10(refFlux1), which is probably no longer correct. I suspect some chunk of that code can be replaced with colorterm.getCorrectedMagnitudes. Does anyone with a bit more familiarity with meas_mosaic want to try it?

            Show
            Parejkoj John Parejko added a comment - Ah... I wonder if the problem is in mosaicTask.py:395 : the code computes magnitudes from the refcat via -2.5*numpy.log10(refFlux1) , which is probably no longer correct. I suspect some chunk of that code can be replaced with colorterm.getCorrectedMagnitudes . Does anyone with a bit more familiarity with meas_mosaic want to try it?
            Hide
            yusra Yusra AlSayyad added a comment -

            The RefLoaders call it thousands of times in singleFrameDriver too:
            /datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156/logs/sfmCG_10156.o151504

            Show
            yusra Yusra AlSayyad added a comment - The RefLoaders call it thousands of times in singleFrameDriver too: /datasets/hsc/repo/rerun/private/yusra/RC2/ DM-10156 /logs/sfmCG_10156.o151504
            Hide
            Parejkoj John Parejko added a comment -

            Yes, I looked at that. I'm not at all surprised about loadSkyCircle being called repeatedly in CalibrateTask: we do everything per CCD, so the refcats would be loaded once per CCD as well. I was surprised about mosaic doing the same, since it should only need to do so once, after all of the data is loaded (that's what jointcal does). But mosaic does a bunch of custom stuff when reading the data.

            Show
            Parejkoj John Parejko added a comment - Yes, I looked at that. I'm not at all surprised about loadSkyCircle being called repeatedly in CalibrateTask : we do everything per CCD, so the refcats would be loaded once per CCD as well. I was surprised about mosaic doing the same, since it should only need to do so once, after all of the data is loaded (that's what jointcal does). But mosaic does a bunch of custom stuff when reading the data.
            Parejkoj John Parejko made changes -
            Link This issue is blocked by DM-16903 [ DM-16903 ]
            Parejkoj John Parejko made changes -
            Link This issue blocks DM-16903 [ DM-16903 ]
            Parejkoj John Parejko made changes -
            Link This issue is blocked by DM-17029 [ DM-17029 ]
            Hide
            Parejkoj John Parejko added a comment - - edited

            Yusra AlSayyad: can you please make a new tickets/DM-10156 stack on lsst-dev for Sophie Reed to test against? I merged DM-16925 and have rebased these so that should make compatibility testing easier. I haven't fixed the meas_mosaic bug above, but I'm going to look into it. It is at the end of the build chain, so will be easy to update once everything else is built.

            Show
            Parejkoj John Parejko added a comment - - edited Yusra AlSayyad : can you please make a new tickets/ DM-10156 stack on lsst-dev for Sophie Reed to test against? I merged DM-16925 and have rebased these so that should make compatibility testing easier. I haven't fixed the meas_mosaic bug above, but I'm going to look into it. It is at the end of the build chain, so will be easy to update once everything else is built.
            Hide
            yusra Yusra AlSayyad added a comment -

            Parejkoj John Parejko added a comment - 2 minutes ago (on DM-16925)
            I've pushed an attempted fix for meas_mosaic (handling the refcat colorterms better): can you please test it?

            Alright I'm going to rebuild tickets/DM-10156 and will rerun the mosaic.py command above with it.

            Show
            yusra Yusra AlSayyad added a comment - Parejkoj John Parejko added a comment - 2 minutes ago (on DM-16925 ) I've pushed an attempted fix for meas_mosaic (handling the refcat colorterms better): can you please test it? Alright I'm going to rebuild tickets/ DM-10156 and will rerun the mosaic.py command above with it.
            swinbank John Swinbank made changes -
            Sprint AP S19-2, AP S19-3, AP S19-4 [ 830, 831, 832 ] AP S19-2, AP S19-3, AP S19-4, AP S19-5 [ 830, 831, 832, 833 ]
            Hide
            yusra Yusra AlSayyad added a comment - - edited

            DIdn't work. This will go faster if you run it off your mosaic ticket branch 

            . /project/yusra/lsstsw3.7/bin/setup.sh
            setup lsst_distrib 
            mosaic.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun private/yusra/RC2/DM-10156:private/parejkoj/DM-10156  --numCoresForReadSource=12 --id ccd=0..8^10..103 visit=1258^1262^1270^1274^1278^1280^1282^1286^1288^1290^1294^1300^1302^1306^1308^1310^1314^1316^1324^1326^1330^24494^24504^24522^24536^24538 tract=9615 --diagnostics --diagDir=/datasets/hsc/repo/rerun/private/private/parejkoj/DM-10156/mosaic_diag/HSC-I --no-versions

            Error I was getting was

            LoadIndexedReferenceObjectsTask WARN: Found version 0 reference catalog with old style units in schema.
            LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy.
            LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details.
            LoadIndexedReferenceObjectsTask INFO: Converted refcat flux fields to nJy (name, units): (g_flux, ''); (r_flux, ''); (i_flux, ''); (z_flux, ''); (y_flux, ''); (i_fluxSigma, ''); (y_fluxSigma, ''); (r_fluxSigma, ''); (z_fluxSigma, ''); (g_fluxSigma, '')
            root WARN: Failed to read {'ccd': 102, 'visit': 1280, 'tract': 9615, 'filter': 'HSC-I'}: 
              File "src/table/BaseRecord.cc", line 76, in void lsst::afw::table::BaseRecord::assign(const lsst::afw::table::BaseRecord&)
                Unequal schemas in record assignment. {0}
            lsst::pex::exceptions::LogicError: 'Unequal schemas in record assignment.'

            Full log in /datasets/hsc/repo/rerun/private/yusra/RC2/DM-10156_v2/logs/mosaic.py

            Show
            yusra Yusra AlSayyad added a comment - - edited DIdn't work. This will go faster if you run it off your mosaic ticket branch  . /project/yusra/lsstsw3. 7 /bin/setup.sh setup lsst_distrib mosaic.py /datasets/hsc/repo --calib /datasets/hsc/repo/CALIB --rerun private /yusra/RC2/DM- 10156 : private /parejkoj/DM- 10156 --numCoresForReadSource= 12 --id ccd= 0 .. 8 ^ 10 .. 103 visit= 1258 ^ 1262 ^ 1270 ^ 1274 ^ 1278 ^ 1280 ^ 1282 ^ 1286 ^ 1288 ^ 1290 ^ 1294 ^ 1300 ^ 1302 ^ 1306 ^ 1308 ^ 1310 ^ 1314 ^ 1316 ^ 1324 ^ 1326 ^ 1330 ^ 24494 ^ 24504 ^ 24522 ^ 24536 ^ 24538 tract= 9615 --diagnostics --diagDir=/datasets/hsc/repo/rerun/ private / private /parejkoj/DM- 10156 /mosaic_diag/HSC-I --no-versions Error I was getting was LoadIndexedReferenceObjectsTask WARN: Found version 0 reference catalog with old style units in schema. LoadIndexedReferenceObjectsTask WARN: run `meas_algorithms/bin/convert_refcat_to_nJy.py` to convert fluxes to nJy. LoadIndexedReferenceObjectsTask WARN: See RFC-575 for more details. LoadIndexedReferenceObjectsTask INFO: Converted refcat flux fields to nJy (name, units): (g_flux, ''); (r_flux, ''); (i_flux, ''); (z_flux, ''); (y_flux, ''); (i_fluxSigma, ''); (y_fluxSigma, ''); (r_fluxSigma, ''); (z_fluxSigma, ''); (g_fluxSigma, '') root WARN: Failed to read {'ccd': 102, 'visit': 1280, 'tract': 9615, 'filter': 'HSC-I'}: File "src/table/BaseRecord.cc", line 76, in void lsst::afw::table::BaseRecord::assign(const lsst::afw::table::BaseRecord&) Unequal schemas in record assignment. {0} lsst::pex::exceptions::LogicError: 'Unequal schemas in record assignment.' Full log in /datasets/hsc/repo/rerun/private/yusra/RC2/ DM-10156 _v2/logs/mosaic.py
            Hide
            Parejkoj John Parejko added a comment -

            Thank you for the details above. I think I've fixed the above (testing on a subset of that data), and I'm running the whole thing now to see what happens.

            Show
            Parejkoj John Parejko added a comment - Thank you for the details above. I think I've fixed the above (testing on a subset of that data), and I'm running the whole thing now to see what happens.
            Hide
            Parejkoj John Parejko added a comment -

            I have fixed the above error, and now the "Before fitting calcChi2" values are the same as the w_2019_10 run, but the fluxFit chi2 printed a bit further down is inf.

            Show
            Parejkoj John Parejko added a comment - I have fixed the above error, and now the "Before fitting calcChi2" values are the same as the w_2019_10 run, but the fluxFit chi2 printed a bit further down is inf .
            Hide
            Parejkoj John Parejko added a comment - - edited

            Success, I think. At least, no infinite chi2 values, and the results look similar to the w_2019_10 run, though not identical. See /home/parejkoj/lsst/meas_mosaic/mosaic-10156.log

            What is the next step here? Chasing rebasing is starting to get tricky.

            Show
            Parejkoj John Parejko added a comment - - edited Success, I think. At least, no infinite chi2 values, and the results look similar to the w_2019_10 run, though not identical. See /home/parejkoj/lsst/meas_mosaic/mosaic-10156.log What is the next step here? Chasing rebasing is starting to get tricky.
            Hide
            yusra Yusra AlSayyad added a comment -

            Hurray!

            Sophie Reed was going to look at the pipe_analysis outputs (to check for surprises in performance) on an HSC RC2 run using DM-10156. Sophie Reed, will you send me the path to the repo with multiband outputs generated with DM-10156?

            Show
            yusra Yusra AlSayyad added a comment - Hurray! Sophie Reed was going to look at the pipe_analysis outputs (to check for surprises in performance) on an HSC RC2 run using DM-10156 . Sophie Reed , will you send me the path to the repo with multiband outputs generated with DM-10156 ?
            yusra Yusra AlSayyad made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            Parejkoj John Parejko added a comment -

            Thank you everyone for the reviews!

            Final post-rebase jenkins run (after merging DM-17029): https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29636/pipeline

            Show
            Parejkoj John Parejko added a comment - Thank you everyone for the reviews! Final post-rebase jenkins run (after merging DM-17029 ): https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29636/pipeline
            Hide
            Parejkoj John Parejko added a comment -

            Huge thank you to all of the reviewers! Got it into this weekly!

            Merged and done (even if Jira doesn't seem to recognize a few of the merged PRs).

            Show
            Parejkoj John Parejko added a comment - Huge thank you to all of the reviewers! Got it into this weekly! Merged and done (even if Jira doesn't seem to recognize a few of the merged PRs).
            Parejkoj John Parejko made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            hchiang2 Hsin-Fang Chiang made changes -
            Link This issue is triggering DM-19015 [ DM-19015 ]
            Parejkoj John Parejko made changes -
            Link This issue is triggering DM-19207 [ DM-19207 ]
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-19265 [ DM-19265 ]

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                Parejkoj John Parejko
                Reviewers:
                Nate Lust, Yusra AlSayyad
                Watchers:
                Chris Morrison, Christopher Waters, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Meredith Rawls, Simon Krughoff, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                10 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel