Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Cotrend Tutorial #137

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

christinahedges
Copy link

Added a tutorial for CBVs on Tabby's star. It's pretty brief, maybe that's a good thing?

@barentsen
Copy link
Member

Is it ready for review?

@christinahedges
Copy link
Author

@barentsen Yes please, it's a bit rough and I'm not sure it's what you wanted...? Early comments are much appreciated! :D Easier to read here.

@mirca
Copy link
Member

mirca commented Oct 26, 2017

screen shot 2017-10-26 at 9 42 34 am

import matplotlib.pyplot as plt
from tqdm import tqdm
from pyke import KeplerCBVCorrector

cbv = KeplerCBVCorrector("kplr008462852-2011073133259_llc.fits")

list_of_cbvs = []
for n in range(1, 16):
    list_of_cbvs.append(range(1, n+1))

neg_log_likelihood = []
for cbvs in tqdm(list_of_cbvs):
    cbv.correct(list(cbvs))
    neg_log_likelihood.append(cbv.opt_result.fun)

n_cbvs = [len(list(list_of_cbvs[i])) for i in range(len(list_of_cbvs))]

plt.plot(n_cbvs, neg_log_likelihood, 'o-.')
plt.xlabel('Number of cbvs')
plt.ylabel('Negative Log Likelihood (Laplacian pdf)')
plt.title("CBV correction Tabby's Star Quarter 8")
plt.show()

Maybe this is going to be helpful to include in this tutorial?

cc @barentsen @christinahedges

@@ -0,0 +1,181 @@

Example 5: Fitting CBVs to Remove Long Term Trends
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GO style is to avoid capitalizing titles, i.e. "remove long term trends" instead of "Remove Long Term Trends".

Example 5: Fitting CBVs to Remove Long Term Trends
==================================================

What are CBVs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a queston mark?

curves. They are built from the most common trends observed in each
channel. You can read more about CBVs in `Demystifying Kepler
Data <https://arxiv.org/pdf/1207.3093.pdf>`__. They can be used to clean
lightcurves of common trends experience by all targets.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to explain CBVs in a lot more detail here. Let's borrow content from Sections 4.2 and 5 of the "Demystifying Kepler Data" (it is fine to copy verbatim from that paper).

@barentsen
Copy link
Member

Thanks for working on this! My main comments are:

  1. Let's add a bunch more content and figures to explain what CBVs are. We can literally copy most of this from the "demystifying Kepler data" paper, which did an excellent job. It is fine to use content from that paper and make it more easily accessible by putting it in the sphinx docs.

  2. I don't think Tabby's star is a good example to demo the CBV-based correction. The figures and appendices in the demystiying Kepler paper show much better examples, let's use/adapt/reproduce those!

  3. I see there is both a ipynb and a rst file. Is one generated from the other? I think we should only have one source file or we'll end up editing the wrong one?

@mirca
Copy link
Member

mirca commented Nov 14, 2017

Yep, let's just use the .ipynb file. Sphinx automagically converts it into .rst for the doc page.

Also, perhaps would be more useful to use the recently added KeplerCBVCorrector class instead of the kepcotrend function?

@barentsen
Copy link
Member

barentsen commented Nov 14, 2017

  • Oh ok, let's remove the rst file from this PR in that case! (Though it's easier to give feedback on, so perhaps it is not a terrible idea to have the rst file in a PR during review and only remove it before merging!)

  • Mixing command-line tools and Python code is going to be confusing for beginners. My proposal is to use only command-line tools in the tutorials, but end every tutorial with a section that says "If you wanted to do the above in a Python script, here is the code:". Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants