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

ValueError: math domain error #27

Closed
iSWORD opened this issue Apr 24, 2022 · 2 comments
Closed

ValueError: math domain error #27

iSWORD opened this issue Apr 24, 2022 · 2 comments

Comments

@iSWORD
Copy link

iSWORD commented Apr 24, 2022

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
Copy link
Owner

abougouffa commented Apr 25, 2022

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:

s = ((dcos(angle)
- dsin(self._conf.latitude) * dsin(delta))
/ (dcos(self._conf.latitude) * dcos(delta)))

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!

return (180 / pi * (atan(-s / sqrt(-s * s + 1)) + pi / 2)) / 15

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:

def _get_asr_time(self):
return self._dohr_time + self._get_time_for_angle(self._get_asr_angle())

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
Copy link
Owner

Duplicate of #19

@abougouffa abougouffa marked this as a duplicate of #19 Apr 25, 2022
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