Skip to content

Jalali isLeap and leapsLength use different leap rules #21

Description

@hosni

In calendars/cjs/jalali.js, leap years are decided with two incompatible algorithms:

  • isLeap(year) uses the 33-year cycle: ((year + 12) % 33) % 4 === 1
  • leapsLength(year) uses the Birashk 2820-year formula

getAllDays() adds leapsLength(year), while month lengths / leap day validity use isLeap(year). Those disagree for most years, so day counts drift relative to the leap table the calendar claims to use.

Reproduction

const jalali = require("./calendars/cjs/jalali");

function countLeapsFromIsLeap(year) {
  let n = 0;
  for (let y = 1; y < year; y++) if (jalali.isLeap(y)) n++;
  return n;
}

console.log(jalali.leapsLength(1403), countLeapsFromIsLeap(1403));
// -> 339, 340

let mismatches = 0;
for (let y = 1; y <= 2000; y++) {
  if (jalali.leapsLength(y) !== countLeapsFromIsLeap(y)) mismatches++;
}
console.log(mismatches);
// -> 1793

Expected

leapsLength(year) should equal the number of leap years in [1, year).
It must use the same rule as isLeap.

Suggested fix

Make leapsLength count with the 33-year cycle (same as isLeap), e.g.:

leapsLength(year) {
  // Number of leap years in [1, year), using the same 33-year rule as isLeap.
  // Closed form of: years y where ((y + 12) % 33) % 4 === 1.
  const n = year - 1;
  return Math.floor(n / 33) * 8 + [0, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 7, 7, 8, 8, 8, 8][n % 33];
}

(Or any equivalent closed form / loop that only calls isLeap.)

This also brings Jalali day-of-epoch counts in line with ICU's Persian calendar for the modern range when paired with the existing epoch.

Impact

Without this, conversions that depend on getAllDays / Julian day can be off by a day versus isLeap-based month length, and versus ICU / civil Persian dates around leap boundaries.

After this fix, Jalali day numbers for modern dates match civil JDN (e.g. 1403-01-01 → 2460390) while Gregorian is still one low until the companion epoch fix.

I think #18 is related to this one!

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions