Michael Wood-Vasey added a comment - 15/Jan/16 12:06 PM This is now down as http://github.com/lsst/validate_drp setup lsst_apps git clone https://github.com/lsst/obs_cfht git clone https://github.com/lsst/validation_data_cfht git clone https://github.com/lsst/validate_drp cd obs_cfht; git checkout 59e204f; setup -r .; scons opt=3; cd - setup -r validation_data_cfht cd validate_drp sh run_test.sh

Hsin-Fang Chiang added a comment - 15/Jan/16 2:32 PM For this ticket review I use and look at the current master of validate_drp. Following README, I was able to run run_test.sh on my machine (OSX 10.10.5) with a slightly old stack (b1817), and got results as follows: 1069 sources in ccd : 12 1083 sources in ccd : 13 1047 sources in ccd : 14 1102 sources in ccd : 21 1110 sources in ccd : 22 1119 sources in ccd : 23 6530 Sources in reference visit : 849375 1601 sources in ccd : 12 3276 sources in ccd : 13 5033 sources in ccd : 14 6748 sources in ccd : 21 8558 sources in ccd : 22 10361 sources in ccd : 23 Visit : 850587 5648 matches found Median value of the astrometric scatter - all magnitudes: 76.0983925574 mas Astrometric scatter (median) - mag < 21 : 22.7943173652 mas I had some comments about README, and later found out that you have a corrected version in the tickets/ DM-4707 branch. So I'll skip those. Note that I haven't looked at other revision in that branch. The validation script valid_cfht.py seems a very good start. I am not sure about the duplication with lsst_dm_stack_demo/bin/check_astrometry . I read your comments in DM-4706 that the duplication is intentional. lsst_dm_stack_demo/bin/check_astrometry appeared to be a more cleaned-up version; work such as copyright, print function, docstring, and other refactoring done there would be nice here as well. But even the version in lsst_dm_stack_demo may use more style clean up to meet our coding standard, such as whilespace, line length, and so on. (I only quickly ran flake8 with it and saw many reminders.)

Hsin-Fang Chiang added a comment - 15/Jan/16 2:43 PM One more thing, the structure of the package seems differernt from other packages. Would it benefit from some structuring?

Michael Wood-Vasey added a comment - 16/Jan/16 8:21 AM Hsin-Fang Chiang Thanks for the review. I very much agree with the structuring suggestions and will keep that in mind as this package continues to evolve. I'll plan on asking you to review DM-4707 shortly, and would very much like to hear more suggestions about organization and code style as we consider that ticket which starts to generalize the analysis to consider more cameras.

Michael Wood-Vasey added a comment - 16/Jan/16 8:24 AM This is done now as of validate_drp 3605f4b Looking back I should have set up a stub on the first master and done d86521c .. 3605f4b as a branch on tickets/ DM-4709 .

Michael Wood-Vasey added a comment - 16/Jan/16 12:34 PM I decided to use a separate set of images for `validate_data_cfht` and `validate_data_decam` because 1) There was still significant discussion about what should be in a test suite for `obs_cfht` 2) While `validate_data_cfht` is currently slightly smaller than `testdata_cfht`, I anticipate that we will increase the size of `validate_data_cfht` soon to add multiple images in a field to be able to calculate photometric repeatability metrics.