We're in the home stretch!

Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29541/pipeline

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 [X]: 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.

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?

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.

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

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

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.

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

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.

The obs packages look fine to me.

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

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).

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.

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).

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.

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?

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

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.

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.

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.

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

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.

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.

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.

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?

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

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).