-
Notifications
You must be signed in to change notification settings - Fork 10
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
sun.rise.set() accuracy and time zone handling #138
Comments
Thanks @clnsmth , that seems like a good solution, and I'm pretty sure the current |
Are current SR SS estimates within an hour of truth for a given location?
…On Tue, Sep 29, 2020 at 2:46 PM Jake Zwart ***@***.***> wrote:
Thanks @clnsmth <https://github.com/clnsmth> , that seems like a good
solution, and I'm pretty sure the current sun.rise.set() functions were
handed down functions for earlier metabolism models. Having backwards
capabilities will be important as changing the sunrise / sunset times will
likely influence metab estimates. Where did you get the NOAA sunrise/sunset
data?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#138 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK6W72NAYK7FA2NYNWUGRDSIITQDANCNFSM4Q67H6RQ>
.
--
Ryan D. Batt, Ph.D.
Aquatic Ecologist turned Data Scientist/ Engineer
Pronouns: he/ him
|
Hi @rBatt ! 👋 |
|
The accuracy of
LakeMetabolizer::sun.rise.set()
is relatively low with respect tosuncalc::getSunlightTimes()
(demonstrated below). Additionally,sun.rise.set()
does not return values in the input tzone whereasgetSunlightTimes()
does. Do these discrepancies warrant changes tosun.rise.set()
?One solution is to nest
sun.rise.set()
in a clause controlled by a newmethod
argument, which defaults to the current implementation but allows a switch to the alternativegetSunlightTimes()
when called upon. An additional ‘lon’ (longitude) argument would be added to meetgetSunlightTimes()
requirements. This solution would maintain backwards compatibility while offering improved accuracy and handling time zones ifsuncalc
is installed (i.e. a suggested library, not a required one). I can submit these proposed changes in a pull request if you’d like.Demo (script is located here):
Create test data (noaa = NOAA, lm = LakeMetabolizer, sc = suncalc)
LakeMetabolizer during standard time
LakeMetabolizer during daylight savings
suncalc during standard time
suncalc during daylight savings
Plot sun rise
Plot sun set
The text was updated successfully, but these errors were encountered: