Chinese calendar follow-up (#3662)

components/calendar/src/astronomy.rs

@@ -56,6 +56,9 @@ pub(crate) const MIN_UTC_OFFSET: f64 = -0.5;
 /// The maximum allowable UTC offset (+14 hours) in fractional days (14.0 / 24.0 days)
 pub(crate) const MAX_UTC_OFFSET: f64 = 14.0 / 24.0;
 
+/// The angle of winter for the purposes of solar calculations
+pub(crate) const WINTER: f64 = 270.0;
+
 impl Location {
     /// Create a location; latitude is from -90 to 90, and longitude is from -180 to 180;
     /// attempting to create a location outside of these bounds will result in a LocationOutOfBoundsError.

components/calendar/src/calendar_arithmetic.rs

@@ -19,6 +19,16 @@ pub struct ArithmeticDate<C: CalendarArithmetic> {
     marker: PhantomData<C>,
 }
 
+/// Maximum number of iterations when iterating through the days of a month; can be increased if necessary
+#[allow(dead_code)] // TODO: Remove dead code tag after use
+pub(crate) const MAX_ITERS_FOR_DAYS_OF_MONTH: u8 = 33;
+
+/// Maximum number of iterations when iterating through the days of a year; can be increased if necessary
+pub(crate) const MAX_ITERS_FOR_DAYS_OF_YEAR: u16 = 370;
+
+/// Maximum number of iterations when iterating through months of a year; can be increased if necessary
+pub(crate) const MAX_ITERS_FOR_MONTHS_OF_YEAR: u8 = 14;
+
 pub trait CalendarArithmetic: Calendar {
     fn month_days(year: i32, month: u8) -> u8;
     fn months_for_every_year(year: i32) -> u8;

components/calendar/src/chinese.rs

@@ -398,7 +398,7 @@ impl Chinese {
     /// let major_solar_term = Chinese::major_solar_term_from_iso(*iso_date.inner());
     /// assert_eq!(major_solar_term, 5);
     /// ```
-    pub fn major_solar_term_from_iso(iso: IsoDateInner) -> i32 {
+    pub fn major_solar_term_from_iso(iso: IsoDateInner) -> u32 {
         let fixed: RataDie = Iso::fixed_from_iso(iso);
         Inner::major_solar_term_from_fixed(fixed)
     }
@@ -414,7 +414,7 @@ impl Chinese {
     /// let minor_solar_term = Chinese::minor_solar_term_from_iso(*iso_date.inner());
     /// assert_eq!(minor_solar_term, 5);
     /// ```
-    pub fn minor_solar_term_from_iso(iso: IsoDateInner) -> i32 {
+    pub fn minor_solar_term_from_iso(iso: IsoDateInner) -> u32 {
         let fixed: RataDie = Iso::fixed_from_iso(iso);
         Inner::minor_solar_term_from_fixed(fixed)
     }
@@ -543,6 +543,30 @@ mod test {
         }
 
         let cases = [
+            TestCase {
+                fixed: -964192,
+                expected_year: -2,
+                expected_month: 1,
+                expected_day: 1,
+            },
+            TestCase {
+                fixed: -963838,
+                expected_year: -1,
+                expected_month: 1,
+                expected_day: 1,
+            },
+            TestCase {
+                fixed: -963129,
+                expected_year: 0,
+                expected_month: 13,
+                expected_day: 1,
+            },
+            TestCase {
+                fixed: -963100,
+                expected_year: 0,
+                expected_month: 13,
+                expected_day: 30,
+            },
             TestCase {
                 fixed: -963099,
                 expected_year: 1,
@@ -598,17 +622,17 @@ mod test {
             assert_eq!(
                 case.expected_year,
                 chinese.year().number,
-                "Chinese from fixed failed for case: {case:?}"
+                "Chinese year from fixed failed for case: {case:?}"
             );
             assert_eq!(
                 case.expected_month,
                 chinese.month().ordinal,
-                "Chinese from fixed failed for case: {case:?}"
+                "Chinese month from fixed failed for case: {case:?}"
             );
             assert_eq!(
                 case.expected_day,
                 chinese.day_of_month().0,
-                "Chinese from fixed failed for case: {case:?}"
+                "Chinese day_of_month from fixed failed for case: {case:?}"
             );
         }
     }

components/calendar/src/chinese_based.rs

@@ -20,8 +20,11 @@
 //! ```
 
 use crate::{
-    astronomy::{Astronomical, Location, MEAN_SYNODIC_MONTH, MEAN_TROPICAL_YEAR},
-    calendar_arithmetic::{ArithmeticDate, CalendarArithmetic},
+    astronomy::{self, Astronomical, Location, MEAN_SYNODIC_MONTH, MEAN_TROPICAL_YEAR},
+    calendar_arithmetic::{
+        ArithmeticDate, CalendarArithmetic, MAX_ITERS_FOR_DAYS_OF_YEAR,
+        MAX_ITERS_FOR_MONTHS_OF_YEAR,
+    },
     helpers::{adjusted_rem_euclid, i64_to_i32, quotient, I32Result},
     rata_die::RataDie,
     types::Moment,
@@ -65,8 +68,7 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
     ///
     /// Based on functions from _Calendrical Calculations_ by Reingold & Dershowitz.
     /// Lisp reference code: https://github.com/EdReingold/calendar-code2/blob/main/calendar.l#L5273-L5281
-    pub(crate) fn major_solar_term_from_fixed(date: RataDie) -> i32 {
-        // TODO: Make this an unsigned int (the size isn't super important, but could fit in a u8)
+    pub(crate) fn major_solar_term_from_fixed(date: RataDie) -> u32 {
         let moment: Moment = date.as_moment();
         let location = C::location(date);
         let universal: Moment = Location::universal_from_standard(moment, location);
@@ -76,7 +78,9 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
             "Solar longitude should be in range of i32"
         );
         let s = solar_longitude.saturate();
-        adjusted_rem_euclid(2 + quotient(s, 30), 12)
+        let result_signed = adjusted_rem_euclid(2 + quotient(s, 30), 12);
+        debug_assert!(result_signed >= 0);
+        result_signed as u32
     }
 
     /// Returns true if the month of a given fixed date does not have a major solar term,
@@ -92,8 +96,8 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
     /// Get the current minor solar term of a fixed date, output as an integer from 1..=12.
     ///
     /// Based on functions from _Calendrical Calculations_ by Reingold & Dershowitz.
-    /// Lisp reference code: https://github.com/EdReingold/calendar-code2/blob/main/calendar.l#L5273-L5281
-    pub(crate) fn minor_solar_term_from_fixed(date: RataDie) -> i32 {
+    /// Lisp reference code: https://github.com/EdReingold/calendar-code2/blob/main/calendar.l#L5303-L5316
+    pub(crate) fn minor_solar_term_from_fixed(date: RataDie) -> u32 {
         let moment: Moment = date.as_moment();
         let location = C::location(date);
         let universal: Moment = Location::universal_from_standard(moment, location);
@@ -103,7 +107,9 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
             "Solar longitude should be in range of i32"
         );
         let s = solar_longitude.saturate();
-        adjusted_rem_euclid(3 + quotient(s - 15, 30), 12)
+        let result_signed = adjusted_rem_euclid(3 + quotient(s - 15, 30), 12);
+        debug_assert!(result_signed >= 0);
+        result_signed as u32
     }
 
     /// The fixed date in standard time at the observation location of the next new moon on or after a given Moment.
@@ -130,8 +136,8 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
     ///
     /// Based on functions from _Calendrical Calculations_ by Reingold & Dershowitz.
     /// Lisp reference code: https://github.com/EdReingold/calendar-code2/blob/main/calendar.l#L5353-L5357
-    pub(crate) fn midnight(date: Moment) -> Moment {
-        Location::universal_from_standard(date, C::location(date.as_rata_die()))
+    pub(crate) fn midnight(moment: Moment) -> Moment {
+        Location::universal_from_standard(moment, C::location(moment.as_rata_die()))
     }
 
     /// Determines the fixed date of the lunar new year in the sui4 (solar year based on the winter solstice)
@@ -165,19 +171,19 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
     /// Lisp reference code: https://github.com/EdReingold/calendar-code2/blob/main/calendar.l#L5359-L5368
     pub(crate) fn winter_solstice_on_or_before(date: RataDie) -> RataDie {
         let approx = Astronomical::estimate_prior_solar_longitude(
-            270.0,
+            astronomy::WINTER,
             Self::midnight((date + 1).as_moment()),
         );
         let mut iters = 0;
-        let max_iters = 367;
         let mut day = Moment::new(libm::floor(approx.inner() - 1.0));
-        while iters < max_iters && 270.0 >= Astronomical::solar_longitude(Self::midnight(day + 1.0))
+        while iters < MAX_ITERS_FOR_DAYS_OF_YEAR
+            && astronomy::WINTER >= Astronomical::solar_longitude(Self::midnight(day + 1.0))
         {
             iters += 1;
             day += 1.0;
         }
         debug_assert!(
-            iters < max_iters,
+            iters < MAX_ITERS_FOR_DAYS_OF_YEAR,
             "Number of iterations was higher than expected"
         );
         day.as_rata_die()
@@ -276,12 +282,11 @@ impl<C: ChineseBased + CalendarArithmetic> ChineseBasedDateInner<C> {
     pub(crate) fn get_leap_month_in_year(date: RataDie) -> u8 {
         let mut cur = Self::new_year_on_or_before_fixed_date(date);
         let mut result = 1;
-        let max_iters = 13;
-        while result < max_iters && !Self::no_major_solar_term(cur) {
+        while result < MAX_ITERS_FOR_MONTHS_OF_YEAR && !Self::no_major_solar_term(cur) {
             cur = Self::new_moon_on_or_after((cur + 1).as_moment());
             result += 1;
         }
-        debug_assert!(result < max_iters, "The given year was not a leap year and an unexpected number of iterations occurred searching for a leap month.");
+        debug_assert!(result < MAX_ITERS_FOR_MONTHS_OF_YEAR, "The given year was not a leap year and an unexpected number of iterations occurred searching for a leap month.");
         result
     }
 }

components/calendar/src/helpers.rs

@@ -353,7 +353,57 @@ pub fn adjusted_rem_euclid(x: i32, y: i32) -> i32 {
     }
 }
 
-/// The value of x shifted into the range (a..b); returns x if a == b; for f64 types
+#[test]
+fn test_adjusted_rem_euclid() {
+    #[derive(Debug)]
+    struct TestCase {
+        x: i32,
+        y: i32,
+        expected: i32,
+    }
+
+    let cases = [
+        TestCase {
+            x: 3,
+            y: 7,
+            expected: 3,
+        },
+        TestCase {
+            x: 7,
+            y: 3,
+            expected: 1,
+        },
+        TestCase {
+            x: -11,
+            y: 9,
+            expected: 7,
+        },
+        TestCase {
+            x: 11,
+            y: 9,
+            expected: 2,
+        },
+        TestCase {
+            x: 11,
+            y: 11,
+            expected: 11,
+        },
+        TestCase {
+            x: -22,
+            y: 11,
+            expected: 11,
+        },
+    ];
+    for case in cases {
+        let result = adjusted_rem_euclid(case.x, case.y);
+        assert_eq!(
+            case.expected, result,
+            "Adjusted rem euclid failed for case: {case:?}"
+        );
+    }
+}
+
+/// The value of x shifted into the range [a..b); returns x if a == b; for f64 types
 pub fn interval_mod_f64(x: f64, a: f64, b: f64) -> f64 {
     if a == b {
         x
@@ -364,8 +414,66 @@ pub fn interval_mod_f64(x: f64, a: f64, b: f64) -> f64 {
 
 #[test]
 fn test_interval_mod() {
-    assert_eq!(interval_mod_f64(5.0, 10.0, 20.0), 15.0);
-    assert_eq!(interval_mod_f64(-5.0, 10.0, 20.0), 15.0);
+    #[derive(Debug)]
+    struct TestCase {
+        x: f64,
+        a: f64,
+        b: f64,
+        expected: f64,
+    }
+
+    let cases = [
+        TestCase {
+            x: 5.0,
+            a: 10.0,
+            b: 20.0,
+            expected: 15.0,
+        },
+        TestCase {
+            x: -5.0,
+            a: 10.0,
+            b: 20.0,
+            expected: 15.0,
+        },
+        TestCase {
+            x: 2.0,
+            a: 12.0,
+            b: 17.0,
+            expected: 12.0,
+        },
+        TestCase {
+            x: 9.0,
+            a: 9.0,
+            b: 10.0,
+            expected: 9.0,
+        },
+        TestCase {
+            x: 16.5,
+            a: 13.5,
+            b: 20.0,
+            expected: 16.5,
+        },
+        TestCase {
+            x: 9.0,
+            a: 3.0,
+            b: 9.0,
+            expected: 3.0,
+        },
+        TestCase {
+            x: 17.0,
+            a: 1.0,
+            b: 5.5,
+            expected: 3.5,
+        },
+    ];
+
+    for case in cases {
+        let result = interval_mod_f64(case.x, case.a, case.b);
+        assert_eq!(
+            case.expected, result,
+            "Interval mod test failed for case: {case:?}"
+        );
+    }
 }
 
 #[test]