search

grd349 / PBjam

A repo for our peak baggin code and tips on jam
MIT License
17 stars 6 forks source link

Star should warn if input periodogram is not flattened or update docs. #197

Closed alexlyttle closed 4 years ago

alexlyttle commented 4 years ago

Context

When I tried passing the following periodogram to the star class,

pg = lc.to_periodogram(normalization='psd')

I got this output from star.run_asy_peakbag():

image

For comparison, when I passed the following periodogram, (flatten() is Lightkurves method to remove noise and smooth the periodogram)

pg = lc.to_periodogram(normalization='psd').flatten()

I get this:

image

Issue

PBjam docs suggests a periodogram with power in any units may be passed, but not that it needs to be flattened. Clearly the periodogram needs to be flattened for this to work, as that would explain the dodgy units in the plot.

Suggestion

  1. (easy) Update the docs to specify the input periodogram must be flattened.
  2. (additional) PBjam raises an error or warning if it detects that the input periodogram is not flattened.
  3. (alternative) star flattens the periodogram if it is not already
nielsenmb commented 4 years ago

I think we should definitely recommend in the docs that the LC be flattened first.

If I recall correctly the periodogram is flattened in session when reading in from text file, but may not be when providing a pg object.

I don't recall putting any flattening in star. It could be a good idea to do so. Although we should check what the effect of flattening multiple times.

Not sure how easy it would be to put in a flatten_detector. While you can easily compute mean/std of the psd, making it robust might be tough.

On Thu, 21 Nov 2019, 15:20 Alex Lyttle, notifications@github.com wrote:

Context

When I tried passing the following periodogram to the star class,

pg = lc.to_periodogram(normalization='psd')

I got this output from star.run_asy_peakbag():

[image: image] https://user-images.githubusercontent.com/43786145/69348676-75c4f780-0c6e-11ea-830f-46967d6d4275.png

For comparison, when I passed the following periodogram, (flatten() is Lightkurves method to remove noise and smooth the periodogram)

pg = lc.to_periodogram(normalization='psd').flatten()

I get this:

[image: image] https://user-images.githubusercontent.com/43786145/69349790-4d3dfd00-0c70-11ea-9558-30256b369a5e.png Issue

PBjam docs suggests a periodogram with power in any units may be passed, but not that it needs to be flattened. Clearly the periodogram needs to be flattened for this to work, as that would explain the dodgy units in the plot. Suggestion

  1. (easy) Update the docs to specify the input periodogram must be flattened.
  2. (additional) PBjam raises an error or warning if it detects that the input periodogram is not flattened.
  3. (alternative) star flattens the periodogram if it is not already

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/issues/197?email_source=notifications&email_token=AEJWO33XGDVT6VI3PPDC3LTQU2RKDA5CNFSM4JQD4ZUKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H3EG4ZQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO32TSPU5ZZQGELZ4QNDQU2RKDANCNFSM4JQD4ZUA .

nielsenmb commented 4 years ago

I've put in a flatten method call in the init of star. Not the best procedure I guess, but it does the trick.

I tested flattening on the same spectrum 10x and it made no measureable difference after the first flattening (does this make sense?). It was a high SNR target, don't know if low SNR targets behave the same.

Tested on 4448777 and it works, feel free to close issue.