tf -> scipy

[aditya-sengupta]

• Takes out dependency on tensorflow, makes eleanor compatible with Python 3.8
• Refactors targetdata.py:psf_lightcurve and expands functionality of models.py

[benmontet]

Hey, awesome! Let me play with it a little bit and try to break it, and assuming I can't I'll merge it in.

Have you done any benchmarking? How does the speed compare to the tf implementation?

[codecov]

Codecov Report

Merging #198 into master will increase coverage by 2.30%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
+ Coverage   46.15%   48.45%   +2.30%     
==========================================
  Files          20       20              
  Lines        2247     2132     -115     
==========================================
- Hits         1037     1033       -4     
+ Misses       1210     1099     -111     

Impacted Files	Coverage Δ
eleanor/models.py	0.00% <0.00%> (ø)
eleanor/targetdata.py	53.92% <7.31%> (+4.88%)	⬆️
eleanor/source.py	49.59% <75.00%> (+0.20%)	⬆️
eleanor/mast.py	88.29% <100.00%> (ø)
eleanor/visualize.py	13.79% <0.00%> (+1.97%)	⬆️
eleanor/update.py	84.18% <0.00%> (+5.81%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2090bc7...d0eac4b. Read the comment docs.

[aditya-sengupta]

Detailed this in an email just now: speed seems about comparable/maybe a little bit slower than tf (unclear how specific to my machine this is). This can be sped up by model updates/the Pytorch upgrade that aren't in this PR but should hopefully be coming soon!

[benmontet]

Hey,

The code looks great! I really appreciate your efforts here.

In testing I'm running in to some issues which are probably due to issues around the initialisation of parameters. I want to make sure the results look the same on your end and my end which will help isolate where the issue is and how to sort it. Can you send me the output of the following from your end?

> star = eleanor.Source(tic=38846515, sector=1)
> data = eleanor.TargetData(star, do_psf=True)
> q = data.quality == 0
> plt.plot(data.time[q], data.psf_flux[q], '.')

I see the following, which is very much not what the star is doing in reality! I'm reasonably confident this is an issue with the initialisation of parameters, with a bad initial guess leading to the minimiser finding a local rather than the global minimum.

Assuming you see the same thing I'll try to trace it through to see if I can find more flexibly sensible starting conditions and I have a feeling everything will go fine from there.

[aditya-sengupta]

Yes, I see a similar result, although I had to delete my earlier cached file and rerun. [image: image.png] Also, I ran into a quick bug around astropy units while I was running this - I think in a recent update they may have enforced unit-aware data processing, so the TPFs for me previously carried units of electron/s, but not this time. This is why I had the quick stopgap of dividing by (u.electron / u.s) (targetdata.py:860, 866, 869) but ideally we could have some slightly more sophisticated processing, i.e. rewriting downstream analysis to work with units and/or do steps differently for the unitless vs. unit-aware versions.

…

On Tue, Sep 29, 2020 at 1:45 AM benmontet ***@***.***> wrote: Hey, The code looks great! I really appreciate your efforts here. In testing I'm running in to some issues which are probably due to issues around the initialisation of parameters. I want to make sure the results look the same on your end and my end which will help isolate where the issue is and how to sort it. Can you send me the output of the following from your end? > star = eleanor.Source(tic=38846515, sector=1) > data = eleanor.TargetData(star, do_psf=True) > q = data.quality == 0 > plt.plot(data.time[q], data.psf_flux[q], '.') I see the following, which is very much not what the star is doing in reality! I'm reasonably confident this is an issue with the initialisation of parameters, with a bad initial guess leading to the minimiser finding a local rather than the global minimum. Assuming you see the same thing I'll try to trace it through to see if I can find more flexibly sensible starting conditions and I have a feeling everything will go fine from there. [image: image] <https://user-images.githubusercontent.com/1314530/94534271-862d0200-0283-11eb-925e-44b31939db44.png> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#198 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIK476TS5H5E63NYBIC3MJ3SIGNDBANCNFSM4RVRRWSQ> .

[aditya-sengupta]

Resolved the aforementioned unit issue