#5 Whisker plots

[HironaoMiyatake]

This pull request is for adding a functionality of generating whisker plots. It includes the following things.

1) A WhiskerPlotSysTest class in sys_tests.py, which is a base class inherited by specific whisker plot classes for three quantitates (g1, g2, and sigma), i.e., 3 classes whose names are WhiskerPlot[Star,PSF,Residual]SysTest.

2) An interface to the HSC pipeline is added in hsc/sys_test_adapters.py. Each test can be run by adding WhiskerPlotStar, WhiskerPlotPSF, WhiskerPlotResidual to a list sys_test in the config. Examples of a command line are as follows

• single CCD

StileCCD.py $SUPRIME_DATA_DIR --id visit=1228 ccd=49 --rerun [rerun_name]:[stile_rerun_name] --clobber-config -c "sys_tests.names=['WhiskerPlotStar', 'WhiskerPlotPSF', 'WhiskerPlotResidual']"

• multiple CCDs

StileVisit.py $SUPRIME_DATA_DIR --id visit=1228 ccd=004^009^090^095 --rerun [rerun_name]:[stile_rerun_name] --clobber-config -c "sys_tests.names=['WhiskerPlotStar', 'WhiskerPlotPSF', 'WhiskerPlotResidual']"

. A bunch of options are added in hsc/base_tasks.py. For getting (x, y) coordinates on a focal plane in the multiple CCD case, a functionality of getting zero point of (x,y) is added to base_task.py.

[rmandelb]

Hi Hironao -

I haven't looked at this code yet, but I have a similar stylistic question as I raised on the other PR: why 3 classes, instead of a single class that has a keyword type that can be either "star", "PSF", or "residual"? One thing that occurs to me now is that perhaps you've made separate classes because that makes a simpler connection to the HSC pipeline outputs?

[msimet]

We do the same thing with the correlation function tests--there's technically a different class for each kind of correlation function, and they differ (at the moment) only by the object types/quantities they request and the specific correlation function used. Here, it's less necessary since the object types are always the same, but on the other hand there's a unity of interface this way (one name/object per test--only the StatSysTests are different). I'm not sure which is better--it seems your vote is for a keyword rather than a separate class?

[HironaoMiyatake]

Right, I just followed the correlation function classes Melanie wrote. I do not have a strong opinion, but we could prepare a single general class which takes quantities we want to look at (such as PSF, star, and residual) as parameters (and these should be configurable). However, it is also important to make things easy for users. For example, we might want to keep WhiskerPlotStar,WhiskerPlotPSF, WhiskerPlotResidual as config options, which internally passes a parameter to look into 'star', 'PSF', and 'residual' to the single general class. This way can provide more freedom to users. For example, for the scatter plots, we might want to keep ScatterPlotStarVsPSFG1, ScatterPlotStarVsPSFG2, and so on, but we can also prepare something like ScatterPlotCustom and then require a user to provide which quantity he/she wants to look at.

However, I think this is out of scope of this PR. Shall we open a new issue for this?

[rmandelb]

I'm not intending to stand in the way of progress, so I'm fine with the idea of making a new issue for it, and evaluating this PR as-is. But we should make sure to get feedback from more people than just us 3 (e.g., Jim). I will open the issue in a moment.

[HironaoMiyatake]

Thank you for opening the issue! I will take a look at the issue...

[msimet]

Hey Hironao, I just looked over this code and it all looks fine to me. I pushed some tweaks to the documentation (nitpicky wording things only), feel free to edit again or undo them if you don't like them. (And I edited a silly punctuation thing I noticed about my own code, too, sorry to clog up your PR!)

One final suggestion that also applies to #18: you should add these to the default list of tests in hsc/base_tasks.py (lines 56 and 601). I think our default right now should be to run everything we can.

[HironaoMiyatake]

Thank you for the tweak! I am happy with this. Shall we merge this to master?

Sorry for the delay on PR of #5 and #18. I was destructed by the HSC collaboration meeting...

[msimet]

I am fine with a merge!

On Thu, Aug 28, 2014 at 6:46 AM, Hironao Miyatake notifications@github.com wrote:

Thank you for the tweak! I am happy with this. Shall we merge this to master?

Sorry for the delay on PR of #5 https://github.com/msimet/Stile/issues/5 and #18 https://github.com/msimet/Stile/issues/18. I was destructed by the HSC collaboration meeting...

— Reply to this email directly or view it on GitHub https://github.com/msimet/Stile/pull/30#issuecomment-53645548.

[rmandelb]

I am also fine with this being merged.

[HironaoMiyatake]

I found that the following error comes from CorrelationFunctionSysTest, when ran the script as default. Could you look into it?

Traceback (most recent call last):
  File "/tigress/HSC/products-20130212/Linux64/pipe_base/HSC-3.0.0/python/lsst/pipe/base/cmdLineTask.py", line 267, in __call__
    result = task.run(dataRef, **kwargs)
  File "/tigress/HSC/users/miyatake/Stile/stile/hsc/base_tasks.py", line 182, in run
    fig = sys_test.sys_test.plot(results)
  File "/tigress/HSC/users/miyatake/Stile/stile/sys_tests.py", line 285, in plot
    plot_data_only &= pd.datarandom_t_field+'d' in fields
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'