batoulapps / adhan-js

High precision Islamic prayer time library for JavaScript
MIT License
376 stars 86 forks source link

Incorrect Isha timing in Milton Canada #78

Closed horus1963 closed 2 years ago

horus1963 commented 2 years ago

Isha timing from adhan.js is different from that from moonsighting.com when selecting the moonsighting.com method. From the site Isha is at 17:59 for Milton (43.494,-79.844) compared to 18:18 from adhan.js. All other timings match exactly.

korbav commented 2 years ago

@horus1963 Could you please provide a fiddle to ease the tests + the expected result (and its origin) and finally the reason why you consider the result produced by adhan-js to be wrong and the other one to be true?

horus1963 commented 2 years ago

Basically, I was comparing the timing for Isha generated by adhan.js (which I installed using npm) when using the moonsighting committee calculation method to that generated by the originator of the moonsighting committee method on moonsighting.com and found a discrepancy. For example for today (Dec 20th 2021), adhan.js reports that isha is at 18:20 whilst on moonsighting.com it is reported as 18:01. Since both should be using the same algorithm, I expect them to be identical. For both the location is as above (43.494,-79.844). Maybe, Khaled at moonsighting.com updated the algorithm since you last adopted it? I have attached a zip of a json of the prayer times generated for my location by moonsighting.com in case it helps with your investigation. Also I am sorry but I do not know what a fiddle is or how to provide it? prayers.json.zip

korbav commented 2 years ago

@horus1963 Thanks for the precision, I accordingly created this to make it clear https://codesandbox.io/s/wild-resonance-p6ofe?file=/src/index.js:226-227

@z3bi I think we're missing something with the Shafaq parameter (AFAIK we don't use this parameter at all) : From my understanding it's only used with the moonsighting method and can take any of these 3 values :

If we follow the official Github referred to by moonsighting.com (https://github.com/islamic-network/prayer-times-moonsighting), we are redirected to the following code in which we can see how they make use of this parameter :

https://github.com/islamic-network/prayer-times/blob/acd29167a6e57566041e82fdde3a31e09978135e/src/PrayerTimes/PrayerTimes.php#L275-L292

https://github.com/islamic-network/prayer-times-moonsighting/tree/master/src/MoonSighting

Last but not least, according to the https://moonsighting.com/how-we.html page, in which they are explaining how they are calculating the prayers times, it seems that they have really fine-grained/empirical rules depending on the latitudes.

So this is unfortunately not a surprise that we are obtaining quite different time values without these adjustments. I'm not sure how often the moonsighting method evolves, but if that's not really frequent, definitely, we should consider to implement it their way.

korbav commented 2 years ago

I created this other sandbox, with an implementation class and I'm having pretty closer results, that seems to be the way to go.

image

I'm not sure where does the 3-minute Isha offset time is coming from though .

@z3bi If you could please take a look https://codesandbox.io/s/affectionate-shaw-5bu5w?file=/src/index.js

If we're good to go with this way the following steps will need to be taken :

z3bi commented 2 years ago

@horus1963 thank you for bringing up this issue, this is an area of the moonsighting method that I've been meaning to implement.

@korbav jazakallah for the proof of concept. I think we would probably want to add a shafaq property to the CalculationParameters object (and document that it is only used for MoonsightingCommittee). I'm not sure why you're getting a 3 minute difference but I will look into that. I have a document from Khalid Shaukat explaining all the calculations. I will go back and reference that document so that we can add this new parameter.

korbav commented 2 years ago

@horus1963 thank you for bringing up this issue, this is an area of the moonsighting method that I've been meaning to implement.

@korbav jazakallah for the proof of concept. I think we would probably want to add a shafaq property to the CalculationParameters object (and document that it is only used for MoonsightingCommittee). I'm not sure why you're getting a 3 minute difference but I will look into that. I have a document from Khalid Shaukat explaining all the calculations. I will go back and reference that document so that we can add this new parameter.

Thank you, that sounds great, let me know if I can help.

z3bi commented 2 years ago

@korbav on my end I'm not getting that three minute difference. InshAllah I will clean up the change and get it ready for a PR. Thank you for the offer to help, I would definitely appreciate a review to double check my work.

korbav commented 2 years ago

@korbav on my end I'm not getting that three minute difference. InshAllah I will clean up the change and get it ready for a PR. Thank you for the offer to help, I would definitely appreciate a review to double check my work.

@z3bi Just to be sure, you mean you don't have this offset with your own implementation or with the above sandbox? I'll definitely review it insha'Allah.

z3bi commented 2 years ago

@z3bi Just to be sure, you mean you don't have this offset with your own implementation or with the above sandbox? I'll definitely review it insha'Allah.

Sorry, I meant with my own implementation.

korbav commented 2 years ago

@z3bi Just to be sure, you mean you don't have this offset with your own implementation or with the above sandbox? I'll definitely review it insha'Allah.

Sorry, I meant with my own implementation.

No worries, it makes sense then, glad that it works flawlessly!

z3bi commented 2 years ago

PR is now available https://github.com/batoulapps/adhan-js/pull/79

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 4.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

horus1963 commented 2 years ago

Thank you very much! Will the guidance apps be updated as well. I own both of the mac and ios versions.

z3bi commented 2 years ago

@horus1963 adding the options to Guidance will take a bit more work. I'm curious what masjid is using the Shafaq Ahmer values for Isha in Milton Canada?

horus1963 commented 2 years ago

Being of mixed ethnicities, there is no consensus between the masjids unfortunately. Some follow the Fiqh council of North America, others don't. Some are Hanafi, others are Shafi, etc. So some mosques follow the Moonsighting Committee whilst others do not want to change from the old North American Method. Therefore we have multiple prayer times, even in the same city. This is very confusing to Muslims, let alone non-Muslims. We do not pray at the same time, nor do we fast at the same time either. Personally, I follow the Shafi school of thought and was taught to follow Shafaq Ahmar. Most Masjids in Milton have Imams originally from Pakistan. They follow the Hanafi School of thought, so they will use Abyad. Some others will use the recommended Shafaq general (recommended by Moonsighting committee). Having said that, they all are usually very flexible and let us pray at the times our school of thought permit. A congregation within a congregation.

z3bi commented 2 years ago

@horus1963 thank you for that explanation, I think having the ability to specify a shafaq type would be useful as an advanced feature but will continue to use the general shafaq as the default value. I will work on adding it to Guidance soon inshAllah.