abougouffa / pyIslam

pyIslam, a Python library to calculate prayer times, hijri date, qiblah direction and more
http://abougouffa.github.io/pyIslam
GNU Lesser General Public License v3.0
94 stars 32 forks source link

ValueError: math domain error #27

Closed iSWORD closed 2 years ago

iSWORD commented 2 years ago

The following inputs:

def test_praytimes_amsterdam():
    # Tested against
    # http://api.aladhan.com/v1/calendarByCity?city=Amsterdam&country=Netherlands&method=3&school=0&month=05&year=2022
    latitude = 52.3745403
    longitude = 4.89797551
    timezone = 2  # GMT+2

    fajr_isha_method = 2  # Muslim World League
    asr_fiqh = 1  # Shafii
    prayer_conf = PrayerConf(longitude, latitude, timezone, fajr_isha_method, asr_fiqh)
    prayer_time = Prayer(prayer_conf, date(2022, 5, 19))

    assert str(prayer_time.fajr_time()) == "03:14:00"
    assert str(prayer_time.sherook_time()) == "05:39:00"
    assert str(prayer_time.dohr_time()) == "13:37:00"
    assert str(prayer_time.asr_time()) == "17:52:00"
    assert str(prayer_time.maghreb_time()) == "21:35:00"
    assert str(prayer_time.ishaa_time()) == "23:52:00"

will result in a ValueError: math domain error inside praytimes@_get_time_for_angle when trying to evaluate sqrt(-s * s + 1) because the value of -s * s + 1 is negative.

I wanted to submit a patch instead of reporting the issue but I have no idea what all the variables represent :(

abougouffa commented 2 years ago

Hello @iSWORD,

Thank you for the feedback.

In fact, this is a known issue for the method I implemented (see issue #19), the term s here: https://github.com/abougouffa/pyIslam/blob/5ed813b67b4414739ef914012e41b4bbad335d10/pyIslam/praytimes.py#L188-L190

Represents the s = cos(diff_time * 15), with delta_time is the time shift to add to the Dohr time in order to calculate times for other prayers. The (atan(-s / sqrt(-s * s + 1)) + pi / 2)) part in the return line should be equivalent to acos(s); Although, I don't remember why I wrote it this way!

https://github.com/abougouffa/pyIslam/blob/5ed813b67b4414739ef914012e41b4bbad335d10/pyIslam/praytimes.py#L191

This time shift is then added to or subtracted from Dohr time to calculate times for other prayers. For example, in the _get_asr_time() method:

https://github.com/abougouffa/pyIslam/blob/5ed813b67b4414739ef914012e41b4bbad335d10/pyIslam/praytimes.py#L256-L257

The issue is, for zones with latitudes beyond [48.5 North, 48.5 South], the used method gives a value for s which is not in the range [-1, 1], this means that the sun is either below the horizon or above the horizon for the whole day (which occurs in the zones beyond [48.5, 48.5S]).

In the issue number #19, I've listed two references which talks about this specific case; however, I didn't find the time to test and implement it. I will plan to do it during my vacation next June.

Thank you again for pointing this issue. Ramadan Mubarak!

abougouffa commented 2 years ago

Duplicate of #19