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

flux-weighted RVs fail if period is exactly 1.0 #4

Open
kecnry opened this issue Apr 20, 2020 · 2 comments
Open

flux-weighted RVs fail if period is exactly 1.0 #4

kecnry opened this issue Apr 20, 2020 · 2 comments

Comments

@kecnry
Copy link

kecnry commented Apr 20, 2020

Using ellc 1.8.5 on Python 2 or 3.

The example in the API docs with a period of 1.5, works:

import ellc
import numpy as np
import matplotlib.pyplot as plt
period = 1.5 
phase = np.arange(-0.25,0.75, 0.001)
time = phase*period
frv1,frv2 = ellc.rv(time,radius_1=0.1,radius_2=0.05,sbratio=0.2,
                    incl=89.95,q=0.5,a=10,ld_1='quad',ldc_1=[0.65,0.2],ld_2='lin',
                    ldc_2=0.45,t_zero=0, period=period,flux_weighted=True)
rv1,rv2 = ellc.rv(time,radius_1=0.1,radius_2=0.05,sbratio=0.2,
                  incl=89.95,q=0.5,a=10,ld_1='quad',ldc_1=[0.65,0.2],ld_2='lin',
                  ldc_2=0.45,t_zero=0, period=period,flux_weighted=False)
plt.plot(phase,frv1)
plt.plot(phase,frv2)
plt.plot(phase,rv1,'--')
plt.plot(phase,rv2,':')
plt.show()

ellc_1 5

but switching the period to 1.0 gives arrays filled with 8.98846567e+307 for frv1 and frv2 (rv1 and rv2 from flux_weighted=False seem fine):

ellc_1 0

periods of 0.99999 or 1.00001 are fine, as are other integers.

@pmaxted
Copy link
Owner

pmaxted commented Apr 20, 2020

Thanks for the bug report but this will not be fixed. There are no in the binaries universe with an orbital period of exactly 1 day, so specifying P=1 means the user is trying to use phase units to calculate the radial velocity. This will not work because the stellar masses are calculated from P and a via's Kepler's law. The documentation is not very clear on this point so I will leave this issue open for now to remind me to update it "soon".

@kecnry
Copy link
Author

kecnry commented Apr 20, 2020

Ok, thanks for the explanation! I can easily work around it, but it had me stumped for a while thinking I was doing something else wrong. So a note in the docs or raising an error when using 1 day would help!

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

No branches or pull requests

2 participants