Skip to content

Since PHP 8, the date_sun_info() function returns inaccurate sunrise and sunset times, but other calculated times are correct #18076

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

Open
JiriJozif opened this issue Mar 15, 2025 · 5 comments

Comments

@JiriJozif
Copy link

Description

The following code:

<?php
date_default_timezone_set("UTC");
$sun_info = date_sun_info(strtotime("2025-03-21"), 51.48, 0.0);
echo date("H:i:s\n", $sun_info['sunrise']);
echo date("H:i:s\n", $sun_info['sunset']);

Resulted in this output PHP 7.4 (compare https://aa.usno.navy.mil/calculated/rstt/oneday?date=2025-03-21&lat=51.48&lon=0.0000&label=&tz=0.00&tz_sign=1&tz_label=false&dst=false&submit=Get+Data):

05:59:21
18:14:48

Resulted in this output PHP 8.3.17:

05:57:45
18:16:25

I think the problem is in the call function timelib_astro_rise_set_altitude() in /ext/date/php_date.c
Back in PHP 7.4.33 it was called like this
timelib_astro_rise_set_altitude(t, longitude, latitude, -35.0/60, 1, &ddummy, &ddummy, &rise, &set, &transit);
https://github.com/php/php-src/blob/PHP-7.4.33/ext/date/php_date.c#L5146
The fourth parameter, -35.0/60, is the refraction value in degrees.
In the timelib_astro_rise_set_altitude() function then subtracts the calculated apparent radius of the Sun (which is approximately 15.0/60)
https://github.com/php/php-src/blob/PHP-7.4.33/ext/date/lib/astro.c#L260 (the variable upper_limb is always 1)
The result is the correct topocentric altitude (-50/60 degrees) needed to calculate the sunrise and sunset times.

However, since PHP 8, it is called timelib_astro_rise_set_altitude(t, longitude, latitude, -50.0/60, 1, &ddummy, &ddummy, &rise, &set, &transit);
https://github.com/php/php-src/blob/PHP-8.0/ext/date/php_date.c#L4663
The fourth parameter, -50.0/60, is refraction and apparent radius together. But in the function itself, the subtraction of the calculated apparent radius of the Sun is still done.
The resulting topocentric altitude (-65/60 degrees) is wrong and consequently so are the calculated sunrise and sunset times. The other calculated times (transit and *twilight*) are correct.

PHP Version

PHP 8.3.17

Operating System

No response

@derickr
Copy link
Member

derickr commented Mar 28, 2025

This got changed by @cmb69 in 7556600 per an incorrect statement in https://bugs.php.net/bug.php?id=65547, where the "apparent radius of the Sun (which is approximately 15.0/60)" is ignored altogether.

I think that change should be reverted.

@carlosbuenosvinos
Copy link
Contributor

carlosbuenosvinos commented Apr 12, 2025

I have run some tests:

  1. The script is returning different values since 8.0 vs. 7.4 and earlier versions (considering the improvement in 7.2 to consider the moon night)

https://3v4l.org/01QlB#v8.4.6

05:57:45
18:16:25

https://3v4l.org/01QlB#v8.0.0

05:57:45
18:16:25

https://3v4l.org/01QlB#v7.4.33

05:59:21
18:14:48
  1. Reverting the change as suggested by @derickr makes the script work as before.
rs = timelib_astro_rise_set_altitude(t, longitude, latitude, -50.0/60, 1, &ddummy, &ddummy, &rise, &set, &transit);
rs = timelib_astro_rise_set_altitude(t, longitude, latitude, -35.0/60, 1, &ddummy, &ddummy, &rise, &set, &transit);

@derickr, double checking, to fix it I understand that I should create the test, PR, and wait for approval, correct?

carlosbuenosvinos added a commit to carlosbuenosvinos/php-src that referenced this issue Apr 12, 2025
@MorganLOCode
Copy link

If I understand the situation rightly, the 50/60 given in the NAO-published algorithm is a 15-minute correction for the angular radius of the sun plus a 35-minute correction for atmospheric refraction.

But as coded the radial correction is already accounted for elsewhere and so shouldn't be included here as well, which means the figure should be 35/60.

Have I got that right?

@JiriJozif
Copy link
Author

Have I got that right?

Yes, that's the correct description.

@carlosbuenosvinos
Copy link
Contributor

PR created: #18317

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants