Debug assertion failures with ECMA-262 Temporal.PlainDate minimum and maximum values

[anba]

There are assertion failures when testing the minimum and maximum allowed Temporal.PlainDate values for the Chinese, Dangi, IslamicObservational, and IslamicUmmAlQura calendars.

I don't know if #4904 will fix these issues, too.

(Tested with release 1.4.)

use icu::calendar::Date;
use icu::calendar::Calendar;

use icu::calendar::chinese::Chinese;
use icu::calendar::dangi::Dangi;
use icu::calendar::islamic::{IslamicObservational, IslamicUmmAlQura};

fn main() {
  // Minimum and maximum dates allowed in ECMA-262 Temporal.
  let min_date_iso = Date::try_new_iso_date(-271821, 4, 19).unwrap();
  let max_date_iso = Date::try_new_iso_date(275760, 9, 13).unwrap();

  // Assertion failure:
  // > Month should be in range of u8! Value -1428 failed for RD RataDie(-99280838)
  {
    let cal = Chinese::new_always_calculating();
    let min_date = cal.date_from_iso(min_date_iso);
    let max_date = cal.date_from_iso(max_date_iso);

    println!("min.year = {}, max.year = {}", cal.year(&min_date).number, cal.year(&max_date).number);
  }

  // Assertion failure:
  // > Month should be in range of u8! Value -1428 failed for RD RataDie(-99280838)
  {
    let cal = Dangi::new_always_calculating();
    let min_date = cal.date_from_iso(min_date_iso);
    let max_date = cal.date_from_iso(max_date_iso);

    println!("min.year = {}, max.year = {}", cal.year(&min_date).number, cal.year(&max_date).number);
  }

  // Assertion failure:
  // > assertion failed: Date::try_new_observational_islamic_date(y, m, d, IslamicObservational::new_always_calculating()).is_ok()
  {
    let cal = IslamicObservational::new_always_calculating();
    let min_date = cal.date_from_iso(min_date_iso);
    let max_date = cal.date_from_iso(max_date_iso);

    println!("min.year = {}, max.year = {}", cal.year(&min_date).number, cal.year(&max_date).number);
  }

  // Assertion failure:
  // > assertion failed: Date::try_new_ummalqura_date(y, m, d, IslamicUmmAlQura::new_always_calculating()).is_ok()
  {
    let cal = IslamicUmmAlQura::new_always_calculating();
    let min_date = cal.date_from_iso(min_date_iso);
    let max_date = cal.date_from_iso(max_date_iso);

    println!("min.year = {}, max.year = {}", cal.year(&min_date).number, cal.year(&max_date).number);
  }
}

[anba]

Still reproduces with release 1.5, except that the assertions are changed:

Chinese/Dangi:

calendrical_calculations-0.1.1/src/chinese_based.rs:571:9:
assertion failed: diff == 29 || diff == 30

IslamicObservational:

calendrical_calculations-0.1.1/src/islamic.rs:92:21:
(ObservationalIslamic) Found year -280804 AH with month length 26 for month 1!

IslamicUmmAlQura:

calendrical_calculations-0.1.1/src/islamic.rs:59:17:
(SaudiIslamic) Found year -280804 AH with length 319!

[sffc]

Yeah, I managed to get these to work for much larger ranges (thousands of years) in #4904, but dates that far away (hundreds of thousands of years) still have precision issues.

A few potential paths forward:

1. Implement more workarounds similar to those in Fix bugs in several calendars with new continuity test #4904 but for the full Temporal range. This would basically mean constructing year/month cutoffs that conform to expectations (29/30 day months) but aren't based on the calendrical calculations directly.
2. Make these calendars fall back to Gregorian years and eras beyond a certain range, such as ±10000 years
3. Make the Islamic calendars fall back to Civil outside the exact range, and make Chinese fall back to some algorithmic metonic cycle approximation outside the exact range

[sffc]

@Manishearth @roozbehp

[Manishearth]

Can we have these assertions not fail outside of ranges where we expect the calendar to be well-behaved?

[sffc]

Start time: 11:05

• @robertbastian - If ECMAScript requires that we behave for 200,000 years, can we use their test cases?
• @sffc - The types of invaraitns that ECMAScript would care about would be the integrity of duration arithmetic and round-tripping, for example.
• @robertbastian - Where did 280,000 years come from?
• @sffc - It is 100 million seconds or days, I think.
• @hsivonen - It seems bad that the calendars would implicitly fall back to a different calendar outside a range. I want to understand why these break. I hear we have precomputed data. Is the math just not valid?
• @Manishearth - There are a lot of problems. The core thing is that these calendars are about ground truth. You have 3 things that all may disagree: what actually happens in space, what your math says happens in space, and what people on the ground think happened in space. These thing may or may not agree. You may happen in cases where the moon rose 1 second after midnight, and it depends on whether you were standing here or there when you saw it. Usually these calendars are published in almanacs and things like that. Beyond a few decades, it's impossible to know the ground truth. And it's pointless to predict in the bast before the existence of the calendar. That said, we can still predict something sensible. It's okay to say, here's what we think the Chinese date will be. It's reasonable to ask, "when would the Chinese new year be in 500 years?" and give them our best approximation to the answer to that question.
• @Manishearth - On the options presented, I wouldn't support falling back to Gregorian for future dates. For past dates, it seems okay. I don't know what the cutoff would be for Islamic; we could fall back to Gregorian for dates before the start of the Hijri era. I'm not sure what it would be for Chinese. Hebrew is the only lunar-like calendar where the math rules over what is in the sky. I would be okay disabling assertions on dates that far in the future.
• @hsivonen - Does Temporal have API surface to say that we fall back to Gregorian or are using inexact calculations? And is this something we should be deciding in ICU4X, or is this something we should ask of Temporal, to put limits on calendars? Presumably ROC and Buddhist go backward and forward as much as Gregorian, right?
• @sffc - Temporal allows returning the era of a year; we could return "gregory" or "pre-gregory" eras for pre-Hijri dates. It doesn't support returning limits on calendars; I think we should stick with returning approximations.
• @robertbastian - If we are basing our implementation on this book, I'm not sure trying to fix this and checking years this far in the future is worth our time. I think falling back to ISO is a good solution.
• @hsivonen - If the cases where this arise are cases that aren't practical use cases, can we get Temporal to say that these calendars do not need to be accurate outside this range?
• @Manishearth - It seems iffy that we guarantee calendar invariants outside a range. We should definitely make sure assertions don't fail.
• @hsivonen - What does ICU4C do?
• @Manishearth - ICU4C doesn't have these assertions. I imagine it might fail the tests.
• @sffc - I think we should make sure continuity test passes for the full Temporal range.
• @sffc - I still prefer Option 3
• @Manishearth - Regardless of what we do here, I think this should be discussed at the spec level.
• @nekevss - I think these invariants might not be required by the spec
• @sffc - (talked about spec being handwavy and also these being invariants he cares about)
• @hsivonen - I do think we should have a Temporal spec that requires interoperable behavior

Options:

1. Try to make calendrical calculations work for large years
2. Switch to Gregorian for out of bounds dates
3. Switch to some hacky metonic cycle
   3a. Do this in a way that no longer requires runtime code (ship more data + metonic hacks) . This would ship all positive Chinese/Hijri dates up to 1k years in the future, and after that use some metonic cycle.
4. Turn off internal calendar invariants for large years. Declare that the calendar does not have Temporal continuity invariants beyond a certain range, and fail tests there. Ask temporal to change the spec.

Proposal: Option 3 or 3a with data size bikeshed tbd after implementation. Temporal issues to be filed upstream motivated by option 4. Ask for Temporal to not be handwavy about this.

Signoff: @sffc @Manishearth @nekevss @echeran (@hsivonen?) (@robertbastian?)

[Manishearth]

Manish to file Temporal issue.

[sffc]

Related to option 3a: might be worth investigating whether Observational and Saudi islamic could share the data cache. Maybe not, because we're already extremely efficient, with 2 bytes per year per calendar.

[Manishearth]

I think that's unlikely but it's possible that there's an efficient way to represent their overlap.

[sffc]

Thanks @Manishearth for filing tc39/proposal-temporal#2869

I found the Temporal invariants that lead to my position that we should enforce certain arithmetic behaviors in calendars we export. See my comment in the above issue.

[sffc]

@hsivonen - What does ICU4C do?
@Manishearth - ICU4C doesn't have these assertions. I imagine it might fail the tests.

Evidence that ICU4C has similar problems but just doesn't have code assertions:

https://unicode-org.atlassian.net/browse/ICU-10866

[anba]

The assertion Year offset too big to store can also be triggered when using years too far into the future or past:

let cal = Chinese::new_always_calculating();
let era = Era::from_str("chinese").unwrap();
let month_code = MonthCode::from_str("M01").unwrap();
let date = cal.date_from_codes(era, 21206, month_code, 1).unwrap();