soroush / libcalendars

Collection of calendar arithmetic algorithms
GNU General Public License v3.0
45 stars 5 forks source link

Wrong Jalali calculation #1

Closed IMAN4K closed 6 years ago

IMAN4K commented 6 years ago

Steps to reproduce

Just use the lib as normal way with the following script:

#include <libcalendars/cl-solar-hijri.h>

#define IRAN_TIME_ZONE_DIFF 12600U // UTC + 03:30(12600s)

float toJD(uint32_t epoch) {
  return (epoch / 86400.0) + 2440587.5;
}

int main(int argc, char *argv[]) {
  uint32_t epoch = 1521318959U; // 20:35:59 PM
  uint32_t JD = toJD(epoch + IRAN_TIME_ZONE_DIFF); // 00:05:59 AM
  int16_t jy;
  uint8_t jm;
  uint16_t jd;
  jdn_to_sh(JD, &jy, &jm, &jd);
  printf("\n Jalali: %d/%d/%d\n", jy, jm, jd);
  system("pause");
  return 0;
}

Expected behaviour

We're 359 second(00:05:59) past the 1396/12/26 and we expect to see 1396/12/27

Actual behaviour

The jdn_to_sh(...) calculate date as 1396/12/26

Configuration

Operating system: Microsoft Windows 10(build 16299.309) Compiler: MSVC2015(14.0) Version of libcalendars: latest git version

Regards. IMAN.

soroush commented 6 years ago

We are using the Chronological Julian Day Number as a basis for our time measurement.

The Chronological Julian Day Number (CJDN) counts the number of whole days since midnight local time at the beginning of January 1st, 4712 BC on the Julian proleptic calendar. You get the Chronological Julian Day Number if you round a Chronological Julian Date (CJD) down to the nearest whole value.

The Chronological Julian Date (CJD) counts the number of days since midnight local time at the beginning of January 1st, −4712 on the Julian proleptic calendar. CJD depends on the local timezone, but JD does not. CJD counts from midnight local time, JD from noon UTC.

So you will need to pass CJDN instead of JDN to all functions in this library.

CJDN = abs(JD + 0.5 + TIMEZONE);

I'll need to update docs to resolve this misunderstanding.

Cheers, Soroush

IMAN4K commented 6 years ago

WELL EXPLAINED. But as the functions name implies, it seems we should provide JDN (e.g jdn_to_sh), but the actual parameter is CJDN. Anyway it would be nice to add epoch compatible APIs (UTC by default) with optional Time zones.

soroush commented 6 years ago

you're right. I'll do that after holidays. Sorry for the inconvenience.

IMAN4K commented 6 years ago

Hi @soroush

Any update on this ? I think some thing's wrong here even with CJDN,in my debugging, i think we're pretty 10800 secs ahead! I've done some test, look at the logs here(the local time and corresponding jalali date):

.
.
1521307799 -> March 17, 2018 5:29:59 PM -> (+3:30) 8:59:59 PM -> Jalali: 1396/12/26
1521307800 -> March 17, 2018 5:30:00 PM -> (+3:30) 9:00:00 PM -> Jalali: 1396/12/27
.
.
1521318599 -> March 17, 2018 8:29:59 PM -> (+3:30) 11:59:59 PM -> Jalali: 1396/12/27
1521318600 -> March 17, 2018 8:30:00 PM -> (+3:30) 12:00:00 AM -> Jalali: 1396/12/27
.
.

The same routine for converting:

float toJD(uint32_t epoch) {
    return (epoch / 86400.0) + 2440587.5;
}

uint32_t epoch;
uint32_t CJDN = abs(toJD(epoch + IRAN_TIME_ZONE_DIFF) + 0.5);
int16_t jy;
uint8_t jm;
uint16_t jd;
jdn_to_sh(CJDN, &jy, &jm, &jd);

Regards. IMAN.

soroush commented 6 years ago

Thanks for reporting the problem.

Seems we have a rounding issue + some glitches in the algorithm. Let me explain with an example. Let's get 2015-01-12T23:59:59+00:00 date as an input. This corresponds to 1421107199 timestamp. Having 1421107199/86400.0 in C (with nominator of type uint32_t and denominator of type float) results in 16448.000000 while it should be 16447.999988425927 which results in having 1393/10/23 instead of 1393/10/22 for 1421107199.

Solutions:

Anyway, in your case using abs doesn't seem to be correct. I will review the algorithm and come back soon, but any sooner I can suggest you to use floor, remove 0.5, and use 2440588 as epoch JDN. This way you will get rid of any timezone calculation after calendaring math. So you will need to:

  1. Add 3.5*60*60 or 4.5*60*60 to the timestamp,
  2. j = floor(timestamp/86400.0f) + 2440588
  3. Use j in jd_to_something functions.