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

Leavitt's Footsteps #21

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

jrka
Copy link
Collaborator

@jrka jrka commented Aug 8, 2023

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:41Z
----------------------------------------------------------------

It might be worth mentioning that we only get back FFIs, since this impacts our later data analysis (no background subtraction)


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:42Z
----------------------------------------------------------------

"Header data unit"


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:43Z
----------------------------------------------------------------

Good to have this function, we should add something above about "to avoid repetitive code, let's create a function to plot cutouts". We should also maybe explain that without vmax/vmin, we're not going to see very much


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:43Z
----------------------------------------------------------------

Line #6.    plt.xlabel('Image Column',fontsize = 14)

As a thought, do we want to add labeling to our function? We could do something like plot_cutout(firstImage, xlabel="Image Column", ylabel="Image Row")

Given that this is a beginner Notebook, I wonder if this would be more or less complicated for the student


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:44Z
----------------------------------------------------------------

citation: https://tess.mit.edu/science/tess-input-catalogue/

(note that as of 28 Aug, the TESS MIT site is down for maintenance)


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:45Z
----------------------------------------------------------------

I had expressed concern about the weird "striations" in the periodograms we generate below

Looking at this plot, it is obvious to me now that those "striations" are due to the "hockey stick" curve of stray light. If we background subtracted (which is likely out-of-scope for this NB), they would go away. Perhaps we should mention this at some point, or create an exercise


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:46Z
----------------------------------------------------------------

Maybe here is the place to discuss striation and stray light? In theory, if we background subtracted these points would line up perfectly


jrka commented on 2023-08-30T04:04:31Z
----------------------------------------------------------------

In addition to the two times this was referenced in the review, I also added reference to it when tpf.interact() is called - very easy to see every pixel getting brighter.

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:47Z
----------------------------------------------------------------

Strange that some of these are normalized and others are not. Did I miss something obvious in the code?


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 28, 2023

View / edit / reply to this conversation on ReviewNB

ttdu commented on 2023-08-28T14:32:49Z
----------------------------------------------------------------

[link to typical HR diagram]


Copy link
Collaborator Author

jrka commented Aug 30, 2023

In addition to the two times this was referenced in the review, I also added reference to it when tpf.interact() is called - very easy to see every pixel getting brighter.


View entire conversation on ReviewNB

@jrka jrka marked this pull request as ready for review August 30, 2023 05:32
@@ -0,0 +1,1937 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

we can link to gaia


Reply via ReviewNB

@@ -0,0 +1,1937 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

14/13?


Reply via ReviewNB

@@ -0,0 +1,1937 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

Line #1.    # Being slow, commented out for now.

I think this was a temporary issue on the MAST side. Seems to be back up to speed now


Reply via ReviewNB

@@ -0,0 +1,850 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

aha. the reason why some of these are normalized and others are not is that the "authors" are different. the QLP (quick-look pipeline) generates normalized light curves; there is no "raw flux" to fall back on.

Ideally, for consistency of data reduction techniques, we would pick one TESS pipeline for all of these light curves. Unfortunately, there are only two "authors" in common for this set of TIC IDs: 'GSFC-ELEANOR-LITE' and 'TGLC'. TGLC isn't supported by lightkurve, and the GSFC curves are pretty "raw", in the sense that they have nonsense values like negative flux

potential solutions

  1. leave it "as-is" and add a disclaimer about the different pipelines
  2. use GSFC and do a "quick and dirty" sigma clip (it produces decent results with.remove_outliers(5))
  3. other solutions at your discretion


Reply via ReviewNB

Copy link
Collaborator

Choose a reason for hiding this comment

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

This just in! Lightkurve 2.4.1 now supports TGLC, so we could actually use this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great, I changed the instructions to use TGLC

@@ -0,0 +1,850 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

Line #3.    for i in range(len(ticlist)):

do we need "i"? we could do something like

for tic in ticlist:

    # Get the obs, keep only the first matching
    obs = lk.search_lightcurve(tic)
    tesslc = obs[0].download()
    
    # Create the periodogram, find period
    pgrm = tesslc.to_periodogram()
    p_max = pgrm.period_at_max_power
    periods.append(p_max.value)
    
    # Plot lc
    tesslc.fold(p_max).scatter(label = f"TIC={tic}, Period = {p_max.value:.3f}d");



Reply via ReviewNB

@@ -0,0 +1,850 @@
{
Copy link
Collaborator

@ttdu ttdu Aug 30, 2023

Choose a reason for hiding this comment

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

the line label didn't print on this (and we should label the axes, as well)


Reply via ReviewNB

Commented back in the query for Gaia data, fixed errors referencing 14 vs 13 length TIC list, fixed errors only saving 7 instead of 8 stars from TIC list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants