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

Wrong prayer times #6

Closed
azzamsa opened this issue Jan 9, 2021 · 7 comments · Fixed by #7
Closed

Wrong prayer times #6

azzamsa opened this issue Jan 9, 2021 · 7 comments · Fixed by #7
Assignees
Labels
bug Something isn't working

Comments

@azzamsa
Copy link

azzamsa commented Jan 9, 2021

Since Ragib's fork is not active anymore. I try to revert all my changes to use insha/salah. Hoping that it will be fixed here.

# I get city coordinates from https://www.mapcoordinates.net/en
let city = Coordinates::new(config.latitude, config.longitude);
let date = Utc::today();    
let params = Configuration::with(Method::Egyptian, Madhab::Shafi);
PrayerTimes::new(date, city, params)

But if you see, with the same config as Ragib's fork. I got very different result. Which is not quite right compared to my city prayer schedule.

   Compiling bilal v0.1.5 (bilal-project/bilal)
    Finished dev [unoptimized + debuginfo] target(s) in 0.67s
     Running `target/debug/bilal -a`
Fajr: 12:55 AM
Sunrise: 12:18 AM
Dhuhr: 4:34 AM
Asr: 7:59 AM
Maghrib: 10:49 AM
Isha: 12:02 PM
Qiyam: 8:14 PM

My city schedule:

image

The time for fajr is 12:55 AM, which the correct one is 03:54 AM

@insha insha self-assigned this Jan 11, 2021
@insha insha added the bug Something isn't working label Jan 11, 2021
@insha
Copy link
Owner

insha commented Jan 11, 2021

@azzamsa What is the city (or coordinates) that you are using? I need those to run through a few tests for this issue and also to make sure the original library is also sound in order to narrow down the issue.

@azzamsa
Copy link
Author

azzamsa commented Jan 12, 2021

Isn't this bug fixed in this comment

The problem with wrong Fajr time is due to a bug in SolarTime::setting_hour that I fixed in my fork with RagibHasin/salah-rs@ea23f0c.


I need those to run through a few tests for this issue

nice idea


Long-Lat:

Central Jakarta (https://www.mapcoordinates.net/en)

latitude: -6.18233995
longitude: 106.84287154

image

Local Prayer Times Schedule (https://www.jadwalsholat.org/)

image

insha/salah Schedule

❯ cargo r -- -a
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/bilal -a`
Fajr: 12:25 AM
Sunrise: 12:47 AM
Dhuhr: 5:02 AM
Asr: 8:27 AM
Maghrib: 11:14 AM
Isha: 12:27 PM
Qiyam: 8:02 PM

comparison

⚠️: jadwalsholat.org has been given: 2 minutes for Ihtiyati (safety) time

Prayer Name insha/salah jadwalsholat.org result
Fajr 12:25 AM 04:15 AM
Sunsrise 12:47 AM 05:45 AM
Duhr 5:02 AM 12:03 AM
Asr 8:27 AM 03:28 PM
Maghrib 11: 14 AM 06:16 PM
Isya 12:27 PM 07:31 PM

Using Ragib's forks, the result will be right. Please take in mind that jadwalsholat.org adds +2 minutes. so without it. Ragib's fork is absolutely close/same.

Prayer Name ragib/salah jadwalsholat.org result
Fajr 4:27 AM 04:15 AM ✔️
Sunsrise 5:47 AM 05:45 AM ✔️
Duhr 12:02 AM 12:03 AM ✔️
Asr 3:27 AM 03:28 PM ✔️
Maghrib 6: 14 AM 06:16 PM ✔️
Isya 07:27 PM 07:31 PM ✔️
❯ cargo r -- -a
    Updating git repository `https://github.com/RagibHasin/salah-rs`
   Compiling salah v0.4.2 (https://github.com/RagibHasin/salah-rs?branch=uptosnuff#fdd27d77)
   Compiling bilal v0.1.5 (/bilal-project/bilal)
    Finished dev [unoptimized + debuginfo] target(s) in 6.09s
     Running `target/debug
Fajr: 4:25 AM
Sunrise: 5:47 AM
Dhuhr: 12:02 PM
Asr: 3:27 PM
Maghrib: 6:14 PM
Isha: 7:27 PM
Qiyam: 1:02 AM

@insha
Copy link
Owner

insha commented Jan 12, 2021

@azzamsa The fork itself cannot be used because it has a lot more changes that are not related to the issue; also that fork does not have any unit tests related to the issue as proof that the fix is a general fix and not associated with a specific location.

That said, I am in the process getting the changes ready for this issue, along with writing tests to confirm the changes are working as expected.

@insha
Copy link
Owner

insha commented Jan 12, 2021

@azzamsa Please confirm the below for prayer times from the above website

  1. Is the time for Fajr listed under "Shubuh"?
  2. Is the time for Sunrise listed under "Terbit"?

@azzamsa
Copy link
Author

azzamsa commented Jan 12, 2021

  • Is the time for Fajr listed under "Shubuh"?

  • Is the time for Sunrise listed under "Terbit"?

Yes. Fajr is Shubuh, and Sunrise is Terbit

insha added a commit that referenced this issue Jan 12, 2021
This was causing the prayer times to be unstable and provide the wrong times for the prayers.

The issue was originally reported by @azzamsa and @RaghibHasin.

Closes #6
insha added a commit that referenced this issue Jan 12, 2021
Tests related to the issue #6.
insha added a commit that referenced this issue Jan 12, 2021
This was causing the prayer times to be unstable and provide the wrong times for the prayers.

The issue was originally reported by @azzamsa and @RaghibHasin.

Closes #6
insha added a commit that referenced this issue Jan 12, 2021
This was causing the prayer times to be unstable and provide the wrong times for the prayers.

The issue was originally reported by @azzamsa and @RaghibHasin.

Closes #6
@insha insha closed this as completed in #7 Jan 12, 2021
@azzamsa
Copy link
Author

azzamsa commented Jan 12, 2021

I still get the wrong result.

Now, I put all the config in one file. So it easy to reproduce.

# main.rs

use salah::prelude::*;

fn main() {
    let central_jakarta = Coordinates::new(-6.18233995, 106.84287154);
    let date = Utc.ymd(2020, 1, 13);
    let params = Configuration::with(Method::Egyptian, Madhab::Shafi);
    let prayers = PrayerSchedule::new()
        .on(date)
        .for_location(central_jakarta)
        .with_configuration(params)
        .calculate();

    match prayers {
        Ok(prayer) => {
            println!(
                "{}: {}",
                Prayer::Fajr.name(),
                prayer.time(Prayer::Fajr).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Sunrise.name(),
                prayer.time(Prayer::Sunrise).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Dhuhr.name(),
                prayer.time(Prayer::Dhuhr).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Asr.name(),
                prayer.time(Prayer::Asr).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Maghrib.name(),
                prayer.time(Prayer::Maghrib).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Isha.name(),
                prayer.time(Prayer::Isha).format("%-l:%M %p").to_string()
            );
            println!(
                "{}: {}",
                Prayer::Qiyam.name(),
                prayer.time(Prayer::Qiyam).format("%-l:%M %p").to_string()
            );
        }
        Err(error) => println!("Could not calculate prayer times: {}", error),
    }
}
❯ cargo r
   Compiling libc v0.2.77
   Compiling num-traits v0.2.12
   Compiling time v0.1.44
   Compiling num-integer v0.1.43
   Compiling chrono v0.4.18
   Compiling salah v0.5.0 (/bilal-project/insha-salah)
   Compiling bilal v0.1.5 (/bilal-project/bilal)
    Finished dev [unoptimized + debuginfo] target(s) in 2.59s
     Running `target/debug/bilal`
Fajr: 9:26 PM
Sunrise: 10:47 PM
Dhuhr: 5:02 AM
Asr: 8:27 AM
Maghrib: 11:15 AM
Isha: 12:27 PM
Qiyam: 6:02 PM

Why fajr is 9:26 PM, the correct one is around ~04:15 AM.

Am I missing something?

@insha
Copy link
Owner

insha commented Jan 13, 2021

@azzamsa Yes, you are missing the bit in the readme about the output being in UTC. The library only returns times in UTC, therefore, all clients will need to take the UTC time and convert it to the appropriate local time. You can take a look at the unit test, calculate_times_for_jakarta(), in the lib.rs for an example of converting the UTC output from the library to the local, to you, Jakarta time.

Repository owner locked as resolved and limited conversation to collaborators Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants