msimet / Stile

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

#45 HSM shapes for galaxies in HSC module #54

Closed msimet closed 9 years ago

msimet commented 9 years ago

This pull request is for the code that allows galaxy shapes to be done with the shape.hsm measurements insstead of shape.sdss when pulling data from the HSC pipeline. At the moment, it does this automatically for any object with galaxy in the object type for the patch and tract (coadd) tests; I didn't see shape.hsm in the schema for the single-visit measurements so it's currently turned off for those, but this is just a boolean config variable do_hsm which is default False for the CCD- and visit-level tests and True for the coadds, so it's easy to change.

This required changing some of the internals of the HSC module by turning all the masks that were passed around into tuples: (mask_type, mask) where mask_type is a string that's the same as the object type. It should be totally transparent to end users; the only thing to be careful about is in sys_test_adapters.py, where if you end up making a custom mask instead of letting the adapter generate one based on the sys_test required objects, then you need to make sure to define a corresponding self.objects_list so the masking routines know what kinds of objects they're masking to.

I think this also addresses some bugs that are in the master-branch code which I hadn't previously realized were there. In particular, it will actually generate shape masks for the Visit and Tract-level tests, which it previously never did thanks to a bug; and I changed the default TreeCorr args for the Patch-level tests, since the radius range was larger than the patches I was looking at for testing. (If I happened to select some odd patches and we do need the larger radius range, please let me know.)

msimet commented 9 years ago

And forgot to say: I didn't see any errors for the sigma parameters in the HSM shape fields, so at the moment it just returns an array of 1s. Any ideas for better solutions gladly accepted.

rmandelb commented 9 years ago

Errors on the size estimates? Those don't have any assigned by the moments code, so I'm not surprised there are none.

A rough estimate would be to use a SNR estimate from the flux. Generally speaking the SNR of the size estimates is comparable to the SNR of the flux estimates, which you can use to get an estimate of errors on sizes.

HironaoMiyatake commented 9 years ago

The change looks nice, but I could not get through the latest pipeline products. I would recommend to go through PR #62 first and merge that to master, because the latest pipeline products no longer work without this fix. What do you think?

msimet commented 9 years ago

Hironao, I think that sounds like a plan for doing PR #62 first...will try to get to that today.

HironaoMiyatake commented 9 years ago

Thanks!

msimet commented 9 years ago

We should probably finish this PR (mostly my fault, I know!).

@HironaoMiyatake 's method up there will transform our e1 and e2 to sky coordinates, but I'm a little nervous about the cutoff at |e|=1. Do we have any other possible ways to do this? (Can we get quantities from the localLinearTransform to do the transformation ourselves, without imposing the cut, or is that a natural outgrowth of any attempt to do the transformation in the linear sense?) Pinging @rmandelb and @TallJimbo here just in case there was something I missed in the email thread we had about this.

If this is something nontrivial to do, we can just use this method for now and then open a new issue for later.

rmandelb commented 9 years ago

If the transformation is dominated by a rotation, then we could do this:

1) check if |e| > 1 2) if so, temporarily rescale it by some factor to make it <1 3) use the transformation method 4) undo the rescaling

But I guess if there is a distortion or something that is dominant, we can�t assume |e| is the same before and after.

HironaoMiyatake commented 9 years ago

Sorry for being silent... But I updated coordinate transformation for HSM shapes. Now we can perform the transformation for |e|>1. I think we can merge this branch now.