msimet / Stile

Stile: the Systematics Tests In Lensing pipeline
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

#32a Single unified generators for the different SysTest types #56

Closed msimet closed 8 years ago

msimet commented 9 years ago

This is a pull request based on issue #32, suggesting that instead of a bunch of different CorrelationFunctionSysTest classes with different names, we have a single generator function or class with a type kwarg. Coding-wise, I went the route of a simple wrapper function that returns instances of the subclasses we'd already made. So all the subclasses still exist (and can be created directly), but this way we have something with a docstring that lists off what all the various names are!

That's the major change here: three new functions called CorrelationFunctionSysTest, WhiskerPlotSysTest, and ScatterPlotSysTest, which are the old class names; the classes are now named BaseCorrelationFunctionSysTest etc. I also changed which things were imported directly into the stile namespace. It's now just those generator functions, rather than all the subclasses (so you can't just tile stile.GalaxyShearSysTest now if you want that class directly--it's stile.sys_tests.GalaxyShearSysTest). I think that's reasonable, since, with this change, I expect all our future example codes will use the generators instead of the subclasses directly. I did not bother to change the internals of the HSC module; if somebody thinks that's a problem for clarity we could switch those over too.

Note that if you want to examine this branch as a whole, it's actually #32a, not #32: I made a first stab at this, didn't like it, and tried again.

HironaoMiyatake commented 9 years ago

Sorry that I am being silent, but I am now looking into this PR. Which pipeline produces do you use for testing the code? I am using the latest pipeline products, and it does not go through Stile because some column went away, etc...

msimet commented 9 years ago

Hi @HironaoMiyatake, do you still have trouble with this branch on the latest pipeline? I've merged in master here which has a bunch of our updates from other branches now...

msimet commented 8 years ago

Hi folks, this PR's been open for a while (9+ months). Anybody wanna take a look at it?

msimet commented 8 years ago

Just pinging this PR so perhaps we can get it merged into master soon...

msimet commented 8 years ago

(Actually, specifically pinging @HironaoMiyatake , who seems to be assigned to this issue!)

I just fixed some things that had been updated since the last merge from master, by the way, in case anyone's tried and failed to look at this branch in the last couple of days.

HironaoMiyatake commented 8 years ago

Sorry I am being silent.... I'll take a look early next week.

HironaoMiyatake commented 8 years ago

I think the code itself is okay, but I could not run Stile on tiger. Could you give it a try?

HironaoMiyatake commented 8 years ago

Stile did not run because files were compressed in .gz. Now it's fixed. However, I got the following error. Could you please take a look, @msimet ?

[miyatake@tiger-sumire reduction_for_galaxy_shape_tests_20151201]$ StileTract.py /tigress/HSC/HSC --rerun=production-20151224:miyatake/production-20151224-test-32a --id tract=9127 filter=HSC-I --clobber-config
2016-02-16T16:02:36: : Config override file does not exist: '/tigress/HSC/products-HSC-20150702/Linux64/obs_subaru/HSC-4.0.0a_hsc/config/TractSingleEpochStile.py'
2016-02-16T16:02:36: : Config override file does not exist: '/tigress/HSC/products-HSC-20150702/Linux64/obs_subaru/HSC-4.0.0a_hsc/config/hsc/TractSingleEpochStile.py'
2016-02-16T16:02:36: : input=/tigress/HSC/HSC/rerun/production-20151224
2016-02-16T16:02:36: : calib=None
2016-02-16T16:02:36: : output=/tigress/HSC/HSC/rerun/miyatake/production-20151224-test-32a
2016-02-16T16:02:36: CameraMapper WARNING: Calibration root directory not found: /tigress/HSC/HSC/rerun/production-20151224/CALIB
2016-02-16T16:02:36: CameraMapper: Loading registry registry from /tigress/HSC/HSC/rerun/miyatake/production-20151224-test-32a/_parent/_parent/_parent/registry.sqlite3
2016-02-16T16:02:36: CameraMapper WARNING: Unable to locate calibRegistry registry in root: /tigress/HSC/HSC/rerun/production-20151224/CALIB/calibRegistry.sqlite3
2016-02-16T16:02:36: CameraMapper WARNING: Unable to locate calibRegistry registry in current dir: ./calibRegistry.sqlite3
2016-02-16T16:02:36: CameraMapper WARNING: No registry loaded; proceeding without one
Traceback (most recent call last):
  File "/tigress/HSC/users/miyatake/Stile/bin/StileTract.py", line 5, in <module>
    TractSingleEpochStileTask.parseAndRun()
  File "/tigress/HSC/products-HSC-20150702/Linux64/pipe_base/HSC-4.0.0/python/lsst/pipe/base/cmdLineTask.py", line 354, in parseAndRun
    resultList = taskRunner.run(parsedCmd)
  File "/tigress/HSC/products-HSC-20150702/Linux64/pipe_base/HSC-4.0.0/python/lsst/pipe/base/cmdLineTask.py", line 158, in run
    resultList = mapFunc(self, self.getTargetList(parsedCmd))
  File "/tigress/HSC/users/miyatake/Stile/stile/hsc/base_tasks.py", line 1223, in __call__
    result = task.run(*args)
  File "/tigress/HSC/users/miyatake/Stile/stile/hsc/base_tasks.py", line 909, in run
    results = sys_test(self.config, *new_catalogs)
  File "/tigress/HSC/users/miyatake/Stile/stile/hsc/sys_test_adapters.py", line 484, in __call__
    return self.sys_test(*new_data, per_ccd_stat=per_ccd_stat)
  File "/tigress/HSC/users/miyatake/Stile/stile/sys_tests.py", line 1550, in __call__
    return super(BaseScatterPlotResidualVsPSFG2SysTest,
NameError: global name 'BaseScatterPlotResidualVsPSFG2SysTest' is not defined
msimet commented 8 years ago

Uh. Oops. That was a bug. :) Does it work now?

HironaoMiyatake commented 8 years ago

Great, it works! I think we can merge the branch into master!

msimet commented 8 years ago

Awesome, thanks!