openEXO / cloud-kepler

Cloud Kepler is a cloud enabled Kepler Planet searching pipeline
8 stars 3 forks source link

We should probably use the proper BLS definition of "depth". #55

Closed scfleming closed 10 years ago

scfleming commented 10 years ago

Currently all three versions of BLS pulse define the depth as the strict absolute value within the transit, but BLS itself defines it as some sort of averaged value.

Step 1.) See if the unit tests will pass if they use the proper BLS version of depth. Step 1b.) If they do not, see if they can be fixed with minimum impact to do so. Step 2.) The BLS pulse algorithms when using real data NEED to use this version of depth (for now), so we might need to add a hidden option when using the unit tests on perfect simulated data to use a different definition.

emprice commented 10 years ago

I can confirm that the unittests will not pass automatically if we change the definition of the depth. At this point, I would say we could either raise the relative tolerance on the depth to ~75% or just not check depth at all...

As to why it doesn't work, I'm pretty sure it's because of the binning. We're passing in "perfect" data, but after the binning, there will probably be some small ingress/egress that makes estimating the depth very difficult, at least in the least-squares sense.

emprice commented 10 years ago

Changes necessary for making this calculation will now reside on the branch dev-bls-depth until this problem can be fixed.

emprice commented 10 years ago

This branch is merged in now.