Adding test for Agadir, Dakhla, Feguig, Fes, Marrakech, Rabat, Tanger

[kafiln]

Assalamu Alaykom

To make sure the Calculation method for Morocco is correct I added more cities in the different regions of Morocco.

What I noticed is that the tests passed for all the cities close to the sea with variance = 1 , Agadir, Casablanca, Dakhla, Rabat, Tanger.

But, for cities far from the sea: Fes, Feguig, Marrakech, Oujda. I had to use variance from 2 to 4

Is there anything we can do the improve the calculation method ?

Thanks

[codecov[bot]]

Codecov Report

Merging #17 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #17   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          418       418           
  Branches        66        66           
=========================================
  Hits           418       418           

Impacted Files	Coverage Δ
src/CalculationMethod.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9a1ed64...cd7293e. Read the comment docs.

[z3bi]

Here are some stats based on the testing data:

Testing Marrakech-Morocco.json (366 days) Difference for Marrakech-Morocco.json - Morocco Average difference: 0.7399817850637522 Max difference: 3.0 fajr Diff - avg: 0.43169398907103823 max: 1.0 sunrise Diff - avg: 1.7704918032786885 max: 3.0 dhuhr Diff - avg: 0.09289617486338798 max: 1.0 asr Diff - avg: 0.3743169398907104 max: 1.0 maghrib Diff - avg: 1.6885245901639345 max: 2.0 isha Diff - avg: 0.08196721311475409 max: 1.0

Testing Tanger-Morocco.json (366 days) Difference for Tanger-Morocco.json - Morocco Average difference: 0.36065573770491804 Max difference: 1.0 fajr Diff - avg: 0.4426229508196721 max: 1.0 sunrise Diff - avg: 0.6038251366120219 max: 1.0 dhuhr Diff - avg: 0.06557377049180328 max: 1.0 asr Diff - avg: 0.39617486338797814 max: 1.0 maghrib Diff - avg: 0.5846994535519126 max: 1.0 isha Diff - avg: 0.07103825136612021 max: 1.0

Testing Dakhla-Morocco.json (366 days) Difference for Dakhla-Morocco.json - Morocco Average difference: 0.20947176684881602 Max difference: 1.0 fajr Diff - avg: 0.3633879781420765 max: 1.0 sunrise Diff - avg: 0.16393442622950818 max: 1.0 dhuhr Diff - avg: 0.13934426229508196 max: 1.0 asr Diff - avg: 0.3633879781420765 max: 1.0 maghrib Diff - avg: 0.08196721311475409 max: 1.0 isha Diff - avg: 0.1448087431693989 max: 1.0

Testing Casablanca-Morocco.json (366 days) Difference for Casablanca-Morocco.json - Morocco Average difference: 0.33970856102003644 Max difference: 1.0 fajr Diff - avg: 0.28688524590163933 max: 1.0 sunrise Diff - avg: 0.33879781420765026 max: 1.0 dhuhr Diff - avg: 0.20218579234972678 max: 1.0 asr Diff - avg: 0.42349726775956287 max: 1.0 maghrib Diff - avg: 0.6475409836065574 max: 1.0 isha Diff - avg: 0.13934426229508196 max: 1.0

Testing Feguig-Morocco.json (366 days) Difference for Feguig-Morocco.json - Morocco Average difference: 1.0947176684881603 Max difference: 4.0 fajr Diff - avg: 0.505464480874317 max: 1.0 sunrise Diff - avg: 3.0628415300546448 max: 4.0 dhuhr Diff - avg: 0.00273224043715847 max: 1.0 asr Diff - avg: 0.3360655737704918 max: 1.0 maghrib Diff - avg: 2.6366120218579234 max: 3.0 isha Diff - avg: 0.02459016393442623 max: 1.0

Testing Oujda-Morocco.json (366 days) Difference for Oujda-Morocco.json - Morocco Average difference: 1.022768670309654 Max difference: 3.0 fajr Diff - avg: 0.3907103825136612 max: 1.0 sunrise Diff - avg: 2.612021857923497 max: 3.0 dhuhr Diff - avg: 0.13387978142076504 max: 1.0 asr Diff - avg: 0.3633879781420765 max: 1.0 maghrib Diff - avg: 2.5054644808743167 max: 3.0 isha Diff - avg: 0.13114754098360656 max: 1.0

Testing Fes-Morocco.json (366 days) Difference for Fes-Morocco.json - Morocco Average difference: 0.7090163934426229 Max difference: 3.0 fajr Diff - avg: 0.5573770491803278 max: 1.0 sunrise Diff - avg: 1.7185792349726776 max: 3.0 dhuhr Diff - avg: 0.01912568306010929 max: 1.0 asr Diff - avg: 0.3770491803278688 max: 1.0 maghrib Diff - avg: 1.510928961748634 max: 2.0 isha Diff - avg: 0.07103825136612021 max: 1.0

Testing Agadir-Morocco.json (366 days) Difference for Agadir-Morocco.json - Morocco Average difference: 0.302367941712204 Max difference: 1.0 fajr Diff - avg: 0.36885245901639346 max: 1.0 sunrise Diff - avg: 0.3114754098360656 max: 1.0 dhuhr Diff - avg: 0.13387978142076504 max: 1.0 asr Diff - avg: 0.37158469945355194 max: 1.0 maghrib Diff - avg: 0.48633879781420764 max: 1.0 isha Diff - avg: 0.14207650273224043 max: 1.0

Testing Rabat-Morocco.json (366 days) Difference for Rabat-Morocco.json - Morocco Average difference: 0.1894353369763206 Max difference: 1.0 fajr Diff - avg: 0.43169398907103823 max: 1.0 sunrise Diff - avg: 0.08196721311475409 max: 1.0 dhuhr Diff - avg: 0.06284153005464481 max: 1.0 asr Diff - avg: 0.40437158469945356 max: 1.0 maghrib Diff - avg: 0.08469945355191257 max: 1.0 isha Diff - avg: 0.07103825136612021 max: 1.0

[z3bi]

We can reduce the variance in Maghrib to a maximum of 2 by removing the static offset and instead using a maghrib angle of 1.8.

I suspect a similar situation is true for Sunrise, instead of using true sunrise they are using an angle below the horizon. The library doesn't support a sunrise angle but it shouldn't be extremely difficult to add and see if that makes the times closer.

[kafiln]

Thank you for your reply.

We can reduce the variance in Maghrib to a maximum of 2 by removing the static offset and instead using a maghrib angle of 1.8.

Did you do some tests or is this a guess ? Do you think it's easy enough for someone like me that does not know how the calculation work to implement it ?

Thanks

[z3bi]

@Kafiil the maghrib angle I validated using the swift version of this library.

I don't think it would be difficult to add the sunrise angle, you can see how the maghrib angle is implemented as a guide.

[kafiln]

If I add a sunrise angle, what value should I use for it ?

[kafiln]

Is this how to implement it ?

if (calculationParameters.sunriseAngle) {
      sunriseTime = new TimeComponents(
        solarTime.hourAngle(-1 * calculationParameters.sunriseAngle, true)
      ).utcDate(date.getFullYear(), date.getMonth(), date.getDate());
    }

[kafiln]

Any comment on this ?

[z3bi]

I would probably implement it like

if (calculationParameters.sunriseAngle) {
            let angleBasedSunrise = new TimeComponents(solarTime.hourAngle(-1 * calculationParameters.sunriseAngle, true)).utcDate(date.getFullYear(), date.getMonth(), date.getDate());
            if (fajrTime < angleBasedSunrise) {
                sunriseTime = angleBasedSunrise;
            }
        }

[z3bi]

Also, using the Swift library it looks like using a sunrise angle of 1.7 degrees and a maghrib angle of 1.8 degrees, you never get a variance larger than 2 minutes. I think that is a reasonably close approximation.

[kafiln]

I implemented as you said but I got bad results. Can you please review my code and help ?

Thanks

[z3bi]

@Kafiil sorry in my quick code sample I put solarTime.hourAngle(-1 * calculationParameters.sunriseAngle, true) but this needs to be solarTime.hourAngle(-1 * calculationParameters.sunriseAngle, false) as the boolean is setting if the angle is a morning angle or an evening angle.

[z3bi]

Also I would ask that this PR not reformat the entire PrayerTimes.js file.

[kafiln]

Thank you for your reply and correction.

Update: I did as advised and indeed I get a maximum variance of 2.

I wanted to make sure this works for all the cities so I decided to add tests for cities that I know have a high altitude, unfortunately the tests did not pass.

I feed I'm close to finding a better solution.

Thank you so much for your help

[z3bi]

@Kafiil how much variance did you find with a high altitude?

[z3bi]

Here is a good explanation of the apparent effects of altitude on sunrise https://astronomy.stackexchange.com/questions/24632/how-do-i-adjust-the-sunrise-equation-to-account-for-elevation

In short, altitude (or elevation) only effects sunrise or sunset if your altitude is higher than the altitude of the horizon (which would allow you to see below the normal horizon). Otherwise if you are simply in a high valley and your altitude and the altitude of the horizon are the same (no matter the height), then the time of sunrise is unaffected by your altitude.

[z3bi]

After looking some more, one additional refinement that can be added is calculating the refraction of the light for a particular angle. We would need to standardize on an assumption of normal atmospheric pressure and a standard temperature, but its worth checking to see how much of a difference atmospheric refraction makes.

[kafiln]

@Kafiil how much variance did you find with a high altitude?

Let me just push my code to see the tests I added

[kafiln]

You can see the updated code now

[z3bi]

@Kafiil I think theres something wrong with the test itself, because when I use the library to get one of the times that fails, I get the correct values.

[z3bi]

@Kafiil the issue is in your test data at the end of the month instead of setting the day to 31 you're setting it back to 1

    {
      "date": "2020-01-30",
      "fajr": "7:19 AM",
      "sunrise": "8:41 AM",
      "dhuhr": "2:16 PM",
      "asr": "5:18 PM",
      "maghrib": "7:43 PM",
      "isha": "8:54 PM"
    },
    {
      "date": "2020-01-01",
      "fajr": "7:19 AM",
      "sunrise": "8:40 AM",
      "dhuhr": "2:16 PM",
      "asr": "5:18 PM",
      "maghrib": "7:44 PM",
      "isha": "8:55 PM"
    },
    {

[kafiln]

Thank you, I will correct that

[z3bi]

@Kafiil let me know when you've fixed the test data and we can merge this in. after that we can release an update on npm.

[kafiln]

@z3bi Sorry I didn't have time lately, I will try to take a look today.

Also, I think I should add some documentation.

Thank you :)

[kafiln]

@z3bi I corrected the data, ( I had a bug in the api 👍 )

Now, I still have cities with variance more than 2, and Ifrane with variance == 5 ( high altitude city Elevation: 5,460 ft (1,665 m))

[z3bi]

@Kafiil these results are complicated. For Ifrane it appears that sunrise and sunset are being calculated as if the observer is at 1,665m and that the horizon is at sea level. This can be verified by adjusting for observer height using the equation here https://en.wikipedia.org/wiki/Sunrise_equation#Hour_angle

For observations on a sea horizon needing an elevation-of-observer correction, add −2.076 x (squareRoot(elevation in meters) / 60) to the −0.83°

This gives us a solar angle of -2.24 and if you use that as the sunrise and sunset angles, the times for Ifrane now match up with a variance of 2.

But if this elevation adjustment is correct, then it needs to be applied to Fajr and Isha as well but we can see that Fajr and Isha times use the same values for Ifrane as they do for other cities.

The other thing to consider is that Ifrane has a coastline to the west, making it possible for the adjustment to Maghrib to be correct. But to the east of Ifrane is additional high altitude terrain. There is no way that sunrise would have the same adjustment.

In the end I am unsure how to reconcile this data.

[kafiln]

Do you think we still can reverse engineer the algorithm used by the Ministry if we have more data ? I Can provide test data for all the cities ( Approximately 105 city)

[z3bi]

@Kafiil so I think I may have found a pattern.

The base value used at 0m elevation (Casablanca, Tanger, Rabat, Agadir) is: fajrAngle: 19.1, sunriseAngle: 1.5, maghribAngle: 1.6, ishaAngle: 17.1

Then they add an adjustment to the sunrise and maghrib angles based on elevation.

For Marrakech (466m) the following values are used: fajrAngle: 19.1, sunriseAngle: 1.8, maghribAngle: 1.9, ishaAngle: 17.1

For Arfoud (807m) the following values are used: fajrAngle: 19.1, sunriseAngle: 2.0, maghribAngle: 2.1, ishaAngle: 17.1

For Ifrane (1,665m) the following values are used: fajrAngle: 19.1, sunriseAngle: 2.3, maghribAngle: 2.4, ishaAngle: 17.1

It's not a linear progression and it doesn't fit elevation adjustment equation mentioned here

Using WolframAlpha you can get quadratic equation that does a good fit WolframAlpha fit but could probably be improved given more data. Perhaps thats something you would be interested in doing.

Thoughts on this

I'm very concerned that this altitude adjustment is being applied to both sunrise and Maghrib times. Based on the terrain of Morocco, the sunrise times would be, if anything, later due to the mountain range to the east. Especially for Marrakech which appears to be at the foot of a mountain range to the east.

I am still also concerned that sunrise and sunset are being adjusted for elevation but not Fajr or Isha. Since they are also based on the appearance of light over the horizon, they should technically follow the same pattern.

And lastly, the way they adjust for elevation doesn't seem to match the standard way to adjust for elevation.

My current inclination is to simply add the "base" value as the Morocco method (fajrAngle: 19.1, sunriseAngle: 1.5, maghribAngle: 1.6, ishaAngle: 17.1) with a note about how elevation is handled for other cities.

[kafiln]

I made a repo that automates the generation of test data.

https://github.com/kafiil/testPrayers

[z3bi]

I made a repo that automates the generation of test data.

https://github.com/kafiil/testPrayers

@Kafiil would it be possible to get a list of altitudes of the different locations? That would help us to try and find a good way to represent the difference in prayer times in the different cities.

[kafiln]

@z3bi I will look if you api I use the get the gps coordinates provides also the altitude

[kafiln]

@z3bi they don't. I will look and add it manually ? Which cities do you need ?

[z3bi]

@Kafiil ideally any locations provided on the habous website that are at a high altitude.

[github-actions[bot]]

Stale pull request message