Michael Wood-Vasey added a comment - 17/Dec/15 11:31 PM Test done and successful. Need to find a place for the `valid_cosmos.py` code to live.

Michael Wood-Vasey added a comment - 15/Jan/16 1:10 PM moved things all into ` http://github.com/lsst/validate_drp ` Will request review once main `validate_drp` for CFHT is approved.

Michael Wood-Vasey added a comment - 19/Jan/16 9:23 PM Moderately quick review Depends on tickets/ DM-4814 to get the data into `validation_data_decam`, so do that one first. Takes perhaps 15 minutes to run the DECam demo. Runs same analysis as the CFHT analysis from DM-4706 , DM-4709 . There's been some refactoring for this generalization to support two survey+cameras. Re-organizing the code to match standard DM packages is deferred to DM-4827 .

Hsin-Fang Chiang added a comment - 22/Jan/16 5:18 PM I haven't run the demo yet due to the git-lfs problem. But so far I found some commit history can be rewritten for clarity. For example, All whitespace changes can be squashed into one The filename run.list got changed to run_cfht.list and then runCfht.list. It doesn't need the middle step. Since the DECam analysis script is a new addition in this ticket, it might as well be added after the generalized check_astrometry.py. Right now a lot got added there and then got deleted. Its name got changed after it was first added too. Similar ideas for changes that were one way and then changed to another within this ticket, as our policy encourages us to "make the commit history look like the code was written correctly the first time." ( https://confluence.lsstcorp.org/display/LDMDG/Git+Commit+Best+Practices )

Michael Wood-Vasey added a comment - 22/Jan/16 7:01 PM Yes. This is the commit history that I agree can be simplified!

Hsin-Fang Chiang added a comment - 23/Jan/16 4:06 PM I was able to run runDecamTest.sh and obtained validating outputs as follows: 5019 sources in ccd: 10 5283 sources in ccd: 11 5192 sources in ccd: 12 5395 sources in ccd: 13 5171 sources in ccd: 14 5831 sources in ccd: 15 5726 sources in ccd: 16 5209 sources in ccd: 17 5428 sources in ccd: 18 48254 total sources in reference visit. Visits : [{ 'filter' : 'z' , 'ccdnum' : 10 , 'visit' : 176846 }] 1104 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 11 , 'visit' : 176846 }] 1198 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 12 , 'visit' : 176846 }] 1211 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 13 , 'visit' : 176846 }] 503 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 14 , 'visit' : 176846 }] 1008 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 15 , 'visit' : 176846 }] 1710 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 16 , 'visit' : 176846 }] 1961 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 17 , 'visit' : 176846 }] 2006 matches found Visits : [{ 'filter' : 'z' , 'ccdnum' : 18 , 'visit' : 176846 }] 1808 matches found Median value of the astrometric scatter - all magnitudes: 116.269301336 mas Astrometric scatter (median) - mag < 21.0 : 63.7 mas Median astrometric scatter 63.7 mas is larger than reference : 25.0 mas I don't understand the purpose of defaultData in checkAstrometry.py . Is it necessary there? If so, please provide docstring. Also, its parameter 'repo' is not used. isExtended in checkAstrometry.py is also missing docstring. Please rewrite and clean up the commit history before merging. Many commits can be squashed, e.g., Rename files to camelCase convention. Rename 'main' -> 'run' Updated README with DECam validation. Run validation on CCDs 10-18; expect >10000 matches. Update Cfht->Decam in runDecamTest example Fix whitespace and script to pass flake8+shellcode Add obs_decam to list of pre-requisites Also see my last comment. I might have missed things, but you get the idea. Beyond the scope of this ticket, I see many measurement warnings like this: processCcdDecam.measurement WARNING: Error in base_Variance.measure on record 75950977596788670 : The center is outside the Footprint of the source record Is this expected?

Michael Wood-Vasey added a comment - 23/Jan/16 5:55 PM Thanks very much for the comments! Added and incorporated.

Michael Wood-Vasey added a comment - 23/Jan/16 5:55 PM Merged to master.