From 5bc584a789acc25bb77e7b4f64c93b66e992d799 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sat, 11 May 2024 13:12:24 -0600 Subject: [PATCH 01/16] Pick up #261 --- src/timeunits.rs | 15 +++++++++++---- tests/epoch.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/timeunits.rs b/src/timeunits.rs index 290d3b4..a9982df 100644 --- a/src/timeunits.rs +++ b/src/timeunits.rs @@ -264,10 +264,17 @@ impl Mul for Unit { } } None => { - if q.is_negative() { - Duration::MIN - } else { - Duration::MAX + // Does not fit on an i64, let's do this again on an 128. + let q = i128::from(q); + match q.checked_mul(factor.into()) { + Some(total_ns) => Duration::from_total_nanoseconds(i128::from(total_ns)), + None => { + if q.is_negative() { + Duration::MIN + } else { + Duration::MAX + } + } } } } diff --git a/tests/epoch.rs b/tests/epoch.rs index 079cd83..701b8f9 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -1999,3 +1999,30 @@ fn regression_test_gh_272() { assert_eq!(day_of_year, 1.0); } } + +#[test] +fn regression_test_gh_261() { + // Validation cases from https://aa.usno.navy.mil/calculated/juliandate?ID=AA&date=1607-01-25&era=AD&time=00%3A00%3A00.000&submit=Get+Date + let validation_cases = &[2308028.5, 2308087.5, 2308240.5, 2308362.5]; + for (mcnt, month) in [1, 3, 8, 12].iter().enumerate() { + for day in 25..=30 { + let epoch = Epoch::from_gregorian_utc(1607, *month, day, 0, 0, 0, 0); + println!("{} - {}", epoch, epoch.to_jde_utc_days()); + + // assert_eq!( + // format!("{epoch}"), + // format!("1607-{month:02}-{day:02}T00:00:00 UTC") + // ); + + // The initial validation cases is 25 days away. + let expected = validation_cases[mcnt] - 25.0 + (day as f64); + + assert_eq!( + epoch.to_jde_utc_days(), + expected, + "err = {}", + epoch.to_jde_utc_days() - expected + ); + } + } +} From c2533795d736dcae8c66d54e0cead6c8d16e88c8 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sat, 11 May 2024 13:20:47 -0600 Subject: [PATCH 02/16] There is a remaining bug in the formatting of old UTC dates --- tests/epoch.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/epoch.rs b/tests/epoch.rs index 701b8f9..4c108f9 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -2007,12 +2007,6 @@ fn regression_test_gh_261() { for (mcnt, month) in [1, 3, 8, 12].iter().enumerate() { for day in 25..=30 { let epoch = Epoch::from_gregorian_utc(1607, *month, day, 0, 0, 0, 0); - println!("{} - {}", epoch, epoch.to_jde_utc_days()); - - // assert_eq!( - // format!("{epoch}"), - // format!("1607-{month:02}-{day:02}T00:00:00 UTC") - // ); // The initial validation cases is 25 days away. let expected = validation_cases[mcnt] - 25.0 + (day as f64); @@ -2023,6 +2017,13 @@ fn regression_test_gh_261() { "err = {}", epoch.to_jde_utc_days() - expected ); + + println!("{} - {}", epoch, epoch.to_jde_utc_days()); + + assert_eq!( + format!("{epoch}"), + format!("1607-{month:02}-{day:02}T00:00:00 UTC") + ); } } } From 9dca4948613a4d84d0ce90d3611b3783a5c75fb2 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Tue, 14 May 2024 15:36:34 -0600 Subject: [PATCH 03/16] Rewrote compute_gregorian. It works for the edge cases but not the nominal cases. I prefer the new approach (less cycles) so I'll work to fix that until I fix the other tests --- src/epoch/gregorian.rs | 55 ++++++++++++++++-------------------------- tests/epoch.rs | 2 -- 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index de05299..9b56444 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -36,8 +36,7 @@ impl Epoch { let (mut year, mut days_in_year) = div_rem_f64(days_f64, DAYS_PER_YEAR_NLD); year += HIFITIME_REF_YEAR; - // Base calculation was on 365 days, so we need to remove one day in seconds per leap year - // between 1900 and `year` + // Base calculation was on 365 days, so we need to remove one day per leap year if year >= HIFITIME_REF_YEAR { for year in HIFITIME_REF_YEAR..year { if is_leap_year(year) { @@ -50,33 +49,21 @@ impl Epoch { days_in_year += 1.0; } } - } - - // Get the month from the exact number of seconds between the start of the year and now - let mut month = 1; - let mut day; - - let mut days_so_far = 0.0; - loop { - let mut days_next_month = usual_days_per_month(month - 1) as f64; - if month == 2 && is_leap_year(year) { - days_next_month += 1.0; + if days_in_year > DAYS_PER_YEAR_NLD { + // We've overflowed the number of days in a year because of the leap years + year += 1; + days_in_year -= DAYS_PER_YEAR_NLD; } + } - if days_so_far + days_next_month > days_in_year || month == 12 { - // We've found the month and can calculate the days - day = if sign >= 0 { - days_in_year - days_so_far + 1.0 - } else { - days_in_year - days_so_far - }; - break; - } + let month_search = CUMULATIVE_DAYS_FOR_MONTH.binary_search(&(days_in_year as u16)); + let mut month = match month_search { + Ok(index) => index + 1, // Exact month found, add 1 for month number (indexing starts from 0) + Err(insertion_point) => insertion_point, // We're before the number of months, so use the insertion point as the month number + }; - // Otherwise, count up the number of days this year so far and keep track of the month. - days_so_far += days_next_month; - month += 1; - } + let mut day = days_in_year - CUMULATIVE_DAYS_FOR_MONTH[month - 1] as f64; + day += 1.0; if hours >= 24 { hours -= 24; @@ -113,19 +100,19 @@ impl Epoch { // Last check on the validity of the Gregorian date - if time == Duration::ZERO || month == 12 && day == 32.0 { - // We've underflowed since we're before 1900. - year += 1; - month = 1; - day = 1.0; - } + // if time == Duration::ZERO || month == 12 && day == 32.0 { + // // We've underflowed since we're before 1900. + // year += 1; + // month = 1; + // day = 1.0; + // } let (_, _, hours, minutes, seconds, milliseconds, microseconds, nanos) = (24 * Unit::Hour + time).decompose(); ( year, - month, + month as u8, day as u8, hours as u8, minutes as u8, @@ -137,7 +124,7 @@ impl Epoch { } else { ( year, - month, + month as u8, day as u8, hours as u8, minutes as u8, diff --git a/tests/epoch.rs b/tests/epoch.rs index 4c108f9..81cd284 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -2018,8 +2018,6 @@ fn regression_test_gh_261() { epoch.to_jde_utc_days() - expected ); - println!("{} - {}", epoch, epoch.to_jde_utc_days()); - assert_eq!( format!("{epoch}"), format!("1607-{month:02}-{day:02}T00:00:00 UTC") From bd2b69111e93e1aa1abaeda75be536e2c37ac287 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Tue, 14 May 2024 16:20:47 -0600 Subject: [PATCH 04/16] Simply the function quite a bit, but some tests still don't pass There's an off by one error in the number of days but I don't know why --- src/epoch/gregorian.rs | 90 +++++++++--------------------------------- tests/epoch.rs | 4 +- 2 files changed, 21 insertions(+), 73 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 9b56444..90035e8 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -24,7 +24,7 @@ impl Epoch { duration: Duration, ts: TimeScale, ) -> (i32, u8, u8, u8, u8, u8, u32) { - let (sign, days, mut hours, minutes, seconds, milliseconds, microseconds, nanos) = + let (sign, days, hours, minutes, seconds, milliseconds, microseconds, nanos) = (duration + ts.gregorian_epoch_offset()).decompose(); let days_f64 = if sign < 0 { @@ -56,8 +56,14 @@ impl Epoch { } } + if is_leap_year(year) && days_in_year > 59.0 { + // NOTE: If on 29th of February, then the day is not finished yet, and therefore + // the extra seconds are added below as per a normal day. + days_in_year -= 1.0; + } + let month_search = CUMULATIVE_DAYS_FOR_MONTH.binary_search(&(days_in_year as u16)); - let mut month = match month_search { + let month = match month_search { Ok(index) => index + 1, // Exact month found, add 1 for month number (indexing starts from 0) Err(insertion_point) => insertion_point, // We're before the number of months, so use the insertion point as the month number }; @@ -65,75 +71,17 @@ impl Epoch { let mut day = days_in_year - CUMULATIVE_DAYS_FOR_MONTH[month - 1] as f64; day += 1.0; - if hours >= 24 { - hours -= 24; - if year >= HIFITIME_REF_YEAR { - day += 1.0; - } else { - day -= 1.0; - } - } - - if day <= 0.0 || days_in_year < 0.0 { - // We've overflowed backward - month = 12; - year -= 1; - // NOTE: Leap year is already accounted for in the TAI duration when counting backward. - day = if days_in_year < 0.0 { - days_in_year + usual_days_per_month(11) as f64 + 1.0 - } else { - usual_days_per_month(11) as f64 - }; - } - - if sign < 0 { - let time = Duration::compose( - sign, - 0, - hours, - minutes, - seconds, - milliseconds, - microseconds, - nanos, - ); - - // Last check on the validity of the Gregorian date - - // if time == Duration::ZERO || month == 12 && day == 32.0 { - // // We've underflowed since we're before 1900. - // year += 1; - // month = 1; - // day = 1.0; - // } - - let (_, _, hours, minutes, seconds, milliseconds, microseconds, nanos) = - (24 * Unit::Hour + time).decompose(); - - ( - year, - month as u8, - day as u8, - hours as u8, - minutes as u8, - seconds as u8, - (nanos - + microseconds * NANOSECONDS_PER_MICROSECOND - + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, - ) - } else { - ( - year, - month as u8, - day as u8, - hours as u8, - minutes as u8, - seconds as u8, - (nanos - + microseconds * NANOSECONDS_PER_MICROSECOND - + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, - ) - } + ( + year, + month as u8, + day as u8, + hours as u8, + minutes as u8, + seconds as u8, + (nanos + + microseconds * NANOSECONDS_PER_MICROSECOND + + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, + ) } #[cfg(feature = "std")] diff --git a/tests/epoch.rs b/tests/epoch.rs index 81cd284..538a1e0 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -1316,7 +1316,7 @@ fn test_timescale_recip() { recip_func(Epoch::from_gregorian_utc(1920, 7, 23, 14, 39, 29, 0)); - recip_func(Epoch::from_gregorian_utc(1954, 12, 24, 6, 6, 31, 0)); + // recip_func(Epoch::from_gregorian_utc(1954, 12, 24, 6, 6, 31, 0)); // Test prior to official leap seconds but with some scaling, valid from 1960 to 1972 according to IAU SOFA. recip_func(Epoch::from_gregorian_utc(1960, 2, 14, 6, 6, 31, 0)); @@ -1333,7 +1333,7 @@ fn test_timescale_recip() { )); // Once every 400 years, there is a leap day on the new century! Joyeux anniversaire, Papa! - recip_func(Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 0)); + // recip_func(Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 0)); recip_func(Epoch::from_gregorian_utc( 2022, From b1ddfe21dcd674650905b582652c607269140d5b Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Wed, 15 May 2024 19:21:27 -0600 Subject: [PATCH 05/16] Still working on Gregorian logic on leap years Also remove commented code --- src/epoch/gregorian.rs | 15 +++++++-------- tests/epoch.rs | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 90035e8..60a364c 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -43,6 +43,11 @@ impl Epoch { days_in_year -= 1.0; } } + if days_in_year < 0.0 { + // We've underflowed the number of days in a year because of the leap years + year -= 1; + days_in_year += DAYS_PER_YEAR_NLD; + } } else { for year in year..HIFITIME_REF_YEAR { if is_leap_year(year) { @@ -57,8 +62,6 @@ impl Epoch { } if is_leap_year(year) && days_in_year > 59.0 { - // NOTE: If on 29th of February, then the day is not finished yet, and therefore - // the extra seconds are added below as per a normal day. days_in_year -= 1.0; } @@ -68,8 +71,8 @@ impl Epoch { Err(insertion_point) => insertion_point, // We're before the number of months, so use the insertion point as the month number }; - let mut day = days_in_year - CUMULATIVE_DAYS_FOR_MONTH[month - 1] as f64; - day += 1.0; + // Directly compute the day from the computed month, and ensure that day counter is one indexed. + let day = days_in_year - CUMULATIVE_DAYS_FOR_MONTH[month - 1] as f64 + 1.0; ( year, @@ -222,8 +225,6 @@ impl Epoch { duration_wrt_ref += Unit::Day; } } - // Remove ref hours from duration to correct for the time scale not starting at midnight - // duration_wrt_ref -= Unit::Hour * time_scale.ref_hour() as i64; } else { // Remove days for year in year..HIFITIME_REF_YEAR { @@ -231,8 +232,6 @@ impl Epoch { duration_wrt_ref -= Unit::Day; } } - // Add ref hours - // duration_wrt_ref += Unit::Hour * time_scale.ref_hour() as i64; } // Add the seconds for the months prior to the current month diff --git a/tests/epoch.rs b/tests/epoch.rs index 538a1e0..81cd284 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -1316,7 +1316,7 @@ fn test_timescale_recip() { recip_func(Epoch::from_gregorian_utc(1920, 7, 23, 14, 39, 29, 0)); - // recip_func(Epoch::from_gregorian_utc(1954, 12, 24, 6, 6, 31, 0)); + recip_func(Epoch::from_gregorian_utc(1954, 12, 24, 6, 6, 31, 0)); // Test prior to official leap seconds but with some scaling, valid from 1960 to 1972 according to IAU SOFA. recip_func(Epoch::from_gregorian_utc(1960, 2, 14, 6, 6, 31, 0)); @@ -1333,7 +1333,7 @@ fn test_timescale_recip() { )); // Once every 400 years, there is a leap day on the new century! Joyeux anniversaire, Papa! - // recip_func(Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 0)); + recip_func(Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 0)); recip_func(Epoch::from_gregorian_utc( 2022, From ece13e9240822336042636b9a5a6ef0dcf83d81a Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Wed, 15 May 2024 19:44:44 -0600 Subject: [PATCH 06/16] Durations print with "day" in singular or plural --- src/duration/mod.rs | 12 ++++++++++-- src/epoch/gregorian.rs | 16 +++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/duration/mod.rs b/src/duration/mod.rs index 7c6e922..ed79a36 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -692,7 +692,15 @@ impl fmt::Display for Duration { } let values = [days, hours, minutes, seconds, milli, us, nano]; - let units = ["days", "h", "min", "s", "ms", "μs", "ns"]; + let units = [ + if days > 1 { "days" } else { "day" }, + "h", + "min", + "s", + "ms", + "μs", + "ns", + ]; let mut insert_space = false; for (val, unit) in values.iter().zip(units.iter()) { @@ -767,7 +775,7 @@ mod ut_duration { fn test_serdes() { for (dt, content) in [ (Duration::from_seconds(10.1), r#""10 s 100 ms""#), - (1.0_f64.days() + 99.nanoseconds(), r#""1 days 99 ns""#), + (1.0_f64.days() + 99.nanoseconds(), r#""1 day 99 ns""#), ( 1.0_f64.centuries() + 99.seconds(), r#""36525 days 1 min 39 s""#, diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 60a364c..765101b 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -61,7 +61,7 @@ impl Epoch { } } - if is_leap_year(year) && days_in_year > 59.0 { + if is_leap_year(year) && days_in_year > CUMULATIVE_DAYS_FOR_MONTH[2] as f64 { days_in_year -= 1.0; } @@ -185,7 +185,9 @@ impl Epoch { } /// Attempts to build an Epoch from the provided Gregorian date and time in the provided time scale. - /// NOTE: If the time scale is TDB, this function assumes that the SPICE format is used + /// + /// Note: + /// The month is ONE indexed, i.e. January is month 1 and December is month 12. #[allow(clippy::too_many_arguments)] pub fn maybe_from_gregorian( year: i32, @@ -592,7 +594,7 @@ pub const fn is_gregorian_valid( nanos: u32, ) -> bool { let max_seconds = if (month == 12 || month == 6) - && day == usual_days_per_month(month - 1) + && day == usual_days_per_month(month) && hour == 23 && minute == 59 && ((month == 6 && july_years(year)) || (month == 12 && january_years(year + 1))) @@ -613,7 +615,7 @@ pub const fn is_gregorian_valid( { return false; } - if day > usual_days_per_month(month - 1) && (month != 2 || !is_leap_year(year)) { + if day > usual_days_per_month(month) && (month != 2 || !is_leap_year(year)) { // Not in February or not a leap year return false; } @@ -651,12 +653,12 @@ const fn july_years(year: i32) -> bool { ) } -/// Returns the usual days in a given month (zero indexed, i.e. January is month zero and December is month 11) +/// Returns the usual days in a given month (ONE indexed, i.e. January is month ONE and December is month 12) /// /// # Warning /// This will return 0 days if the month is invalid. const fn usual_days_per_month(month: u8) -> u8 { - match month + 1 { + match month { 1 | 3 | 5 | 7 | 8 | 10 | 12 => 31, 4 | 6 | 9 | 11 => 30, 2 => 28, @@ -669,7 +671,7 @@ const CUMULATIVE_DAYS_FOR_MONTH: [u16; 12] = { let mut days = [0; 12]; let mut month = 1; while month < 12 { - days[month] = days[month - 1] + usual_days_per_month(month as u8 - 1) as u16; + days[month] = days[month - 1] + usual_days_per_month(month as u8) as u16; month += 1; } days From d5b9ebf5c0051073cac7e3d1a9e891bb2127da23 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Wed, 15 May 2024 19:53:10 -0600 Subject: [PATCH 07/16] Add CUMULATIVE_DAYS_FOR_MONTH_LEAP_YEARS constant --- src/epoch/gregorian.rs | 40 +++++++++++++++++++++++++++++----------- tests/epoch.rs | 12 ++++++------ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 765101b..dcbfb84 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -61,18 +61,20 @@ impl Epoch { } } - if is_leap_year(year) && days_in_year > CUMULATIVE_DAYS_FOR_MONTH[2] as f64 { - days_in_year -= 1.0; - } + let cumul_days = if is_leap_year(year) { + CUMULATIVE_DAYS_FOR_MONTH_LEAP_YEARS + } else { + CUMULATIVE_DAYS_FOR_MONTH + }; - let month_search = CUMULATIVE_DAYS_FOR_MONTH.binary_search(&(days_in_year as u16)); + let month_search = cumul_days.binary_search(&(days_in_year as u16)); let month = match month_search { Ok(index) => index + 1, // Exact month found, add 1 for month number (indexing starts from 0) Err(insertion_point) => insertion_point, // We're before the number of months, so use the insertion point as the month number }; // Directly compute the day from the computed month, and ensure that day counter is one indexed. - let day = days_in_year - CUMULATIVE_DAYS_FOR_MONTH[month - 1] as f64 + 1.0; + let day = days_in_year - cumul_days[month - 1] as f64 + 1.0; ( year, @@ -237,13 +239,15 @@ impl Epoch { } // Add the seconds for the months prior to the current month - duration_wrt_ref += Unit::Day * i64::from(CUMULATIVE_DAYS_FOR_MONTH[(month - 1) as usize]); + let cumul_days = if is_leap_year(year) { + CUMULATIVE_DAYS_FOR_MONTH_LEAP_YEARS + } else { + CUMULATIVE_DAYS_FOR_MONTH + }; - if is_leap_year(year) && month > 2 { - // NOTE: If on 29th of February, then the day is not finished yet, and therefore - // the extra seconds are added below as per a normal day. - duration_wrt_ref += Unit::Day; - } + // Add the number of days based on the input month + duration_wrt_ref += Unit::Day * i64::from(cumul_days[(month - 1) as usize]); + // Add the number of days based on the input day and time. duration_wrt_ref += Unit::Day * i64::from(day - 1) + Unit::Hour * i64::from(hour) + Unit::Minute * i64::from(minute) @@ -677,6 +681,20 @@ const CUMULATIVE_DAYS_FOR_MONTH: [u16; 12] = { days }; +/// Calculates the prefix-sum of days counted up to the month start, for leap years only +const CUMULATIVE_DAYS_FOR_MONTH_LEAP_YEARS: [u16; 12] = { + let mut days = [0; 12]; + let mut month = 1; + while month < 12 { + days[month] = days[month - 1] + usual_days_per_month(month as u8) as u16; + if month == 2 { + days[month] += 1; + } + month += 1; + } + days +}; + /// `is_leap_year` returns whether the provided year is a leap year or not. /// Tests for this function are part of the Datetime tests. pub(crate) const fn is_leap_year(year: i32) -> bool { diff --git a/tests/epoch.rs b/tests/epoch.rs index 81cd284..aadbda7 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -1855,14 +1855,14 @@ fn test_epoch_formatter() { let bday = Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 37); - let fmt_iso_ord = Formatter::new(bday, ISO8601_ORDINAL); - assert_eq!(format!("{fmt_iso_ord}"), "2000-060"); + // let fmt_iso_ord = Formatter::new(bday, ISO8601_ORDINAL); + // assert_eq!(format!("{fmt_iso_ord}"), "2000-060"); - let fmt_iso_ord = Formatter::new(bday, Format::from_str("%j").unwrap()); - assert_eq!(format!("{fmt_iso_ord}"), "060"); + // let fmt_iso_ord = Formatter::new(bday, Format::from_str("%j").unwrap()); + // assert_eq!(format!("{fmt_iso_ord}"), "060"); - let fmt_iso = Formatter::new(bday, ISO8601); - assert_eq!(format!("{fmt_iso}"), format!("{bday}")); + // let fmt_iso = Formatter::new(bday, ISO8601); + // assert_eq!(format!("{fmt_iso}"), format!("{bday}")); let fmt = Formatter::new(bday, ISO8601_DATE); assert_eq!(format!("{fmt}"), format!("2000-02-29")); From 938204b89635d6b70d2e49137c9ef13fd8a271bc Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Wed, 15 May 2024 22:39:56 -0600 Subject: [PATCH 08/16] Removed edges cases in Gregorian computation for very negative dates --- src/epoch/gregorian.rs | 60 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index dcbfb84..4dab0f2 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -35,6 +35,11 @@ impl Epoch { let (mut year, mut days_in_year) = div_rem_f64(days_f64, DAYS_PER_YEAR_NLD); year += HIFITIME_REF_YEAR; + if sign == -1 { + // We count backward, and reference time is zero days, so we remove one day in the year here. + // This looks hacky but it works quite well to avoid lots of edge cases. + days_in_year -= 1.0; + } // Base calculation was on 365 days, so we need to remove one day per leap year if year >= HIFITIME_REF_YEAR { @@ -76,17 +81,50 @@ impl Epoch { // Directly compute the day from the computed month, and ensure that day counter is one indexed. let day = days_in_year - cumul_days[month - 1] as f64 + 1.0; - ( - year, - month as u8, - day as u8, - hours as u8, - minutes as u8, - seconds as u8, - (nanos - + microseconds * NANOSECONDS_PER_MICROSECOND - + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, - ) + if sign == -1 { + // Our date calculation is independant from the time in that day, so we fix it here. + + // Recompute the time since we count backward and not forward for negative durations. + let time = 24 * Unit::Hour + - Duration::compose( + 0, + 0, + hours, + minutes, + seconds, + milliseconds, + microseconds, + nanos, + ); + + // Compute the correct time. + let (_, _, hours, minutes, seconds, milliseconds, microseconds, nanos) = + time.decompose(); + + ( + year, + month as u8, + day as u8, + hours as u8, + minutes as u8, + seconds as u8, + (nanos + + microseconds * NANOSECONDS_PER_MICROSECOND + + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, + ) + } else { + ( + year, + month as u8, + day as u8, + hours as u8, + minutes as u8, + seconds as u8, + (nanos + + microseconds * NANOSECONDS_PER_MICROSECOND + + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, + ) + } } #[cfg(feature = "std")] From 28b91fbc3ac3e7abde97cdb3ba53583ef74933ed Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 16:15:34 -0600 Subject: [PATCH 09/16] Trying to fix the to/from Gregorian --- src/epoch/gregorian.rs | 43 +++++++++++++++++++++++------------------- src/epoch/mod.rs | 9 +++++++-- tests/epoch.rs | 42 +++++++++++++++++++++++------------------ 3 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 4dab0f2..a0bf7d8 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -22,29 +22,29 @@ use super::div_rem_f64; impl Epoch { pub(crate) fn compute_gregorian( duration: Duration, - ts: TimeScale, + time_scale: TimeScale, ) -> (i32, u8, u8, u8, u8, u8, u32) { + let duration_wrt_ref = duration + time_scale.gregorian_epoch_offset(); let (sign, days, hours, minutes, seconds, milliseconds, microseconds, nanos) = - (duration + ts.gregorian_epoch_offset()).decompose(); + duration_wrt_ref.decompose(); + + let rslt = duration_wrt_ref.to_unit(Unit::Day); + dbg!(rslt); let days_f64 = if sign < 0 { - -(days as f64) + // This looks hacky but it avoids lots of edge cases. + -((days + 1) as f64) } else { days as f64 }; let (mut year, mut days_in_year) = div_rem_f64(days_f64, DAYS_PER_YEAR_NLD); year += HIFITIME_REF_YEAR; - if sign == -1 { - // We count backward, and reference time is zero days, so we remove one day in the year here. - // This looks hacky but it works quite well to avoid lots of edge cases. - days_in_year -= 1.0; - } // Base calculation was on 365 days, so we need to remove one day per leap year if year >= HIFITIME_REF_YEAR { - for year in HIFITIME_REF_YEAR..year { - if is_leap_year(year) { + for y in HIFITIME_REF_YEAR..year { + if is_leap_year(y) { days_in_year -= 1.0; } } @@ -54,12 +54,14 @@ impl Epoch { days_in_year += DAYS_PER_YEAR_NLD; } } else { - for year in year..HIFITIME_REF_YEAR { - if is_leap_year(year) { + for y in year..HIFITIME_REF_YEAR { + if is_leap_year(y) { days_in_year += 1.0; } } - if days_in_year > DAYS_PER_YEAR_NLD { + if (days_in_year > DAYS_PER_YEAR_NLD && !is_leap_year(year)) + || (days_in_year > DAYS_PER_YEAR_NLD + 1.0 && is_leap_year(year)) + { // We've overflowed the number of days in a year because of the leap years year += 1; days_in_year -= DAYS_PER_YEAR_NLD; @@ -249,7 +251,7 @@ impl Epoch { source: DurationError::Underflow, }) } - Some(years_since_ref) => match years_since_ref.checked_mul(365) { + Some(years_since_ref) => match years_since_ref.checked_mul(DAYS_PER_YEAR_NLD as i32) { None => { return Err(EpochError::Duration { source: DurationError::Overflow, @@ -257,20 +259,20 @@ impl Epoch { } Some(days) => Unit::Day * i64::from(days), }, - } - time_scale.gregorian_epoch_offset(); + }; // Now add the leap days for all the years prior to the current year if year >= HIFITIME_REF_YEAR { // Add days - for year in HIFITIME_REF_YEAR..year { - if is_leap_year(year) { + for y in HIFITIME_REF_YEAR..year { + if is_leap_year(y) { duration_wrt_ref += Unit::Day; } } } else { // Remove days - for year in year..HIFITIME_REF_YEAR { - if is_leap_year(year) { + for y in year..HIFITIME_REF_YEAR { + if is_leap_year(y) { duration_wrt_ref -= Unit::Day; } } @@ -298,6 +300,9 @@ impl Epoch { duration_wrt_ref -= Unit::Second; } + // Account for this time scale's Gregorian offset. + duration_wrt_ref -= time_scale.gregorian_epoch_offset(); + Ok(Self { duration: duration_wrt_ref, time_scale, diff --git a/src/epoch/mod.rs b/src/epoch/mod.rs index e2bd5f1..8c34ba0 100644 --- a/src/epoch/mod.rs +++ b/src/epoch/mod.rs @@ -1370,9 +1370,14 @@ fn div_rem_f64(me: f64, rhs: f64) -> (i32, f64) { fn div_euclid_f64(lhs: f64, rhs: f64) -> f64 { let q = (lhs / rhs).trunc(); if lhs % rhs < 0.0 { - return if rhs > 0.0 { q - 1.0 } else { q + 1.0 }; + if rhs > 0.0 { + q - 1.0 + } else { + q + 1.0 + } + } else { + q } - q } fn rem_euclid_f64(lhs: f64, rhs: f64) -> f64 { diff --git a/tests/epoch.rs b/tests/epoch.rs index aadbda7..49a1b34 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -2004,24 +2004,30 @@ fn regression_test_gh_272() { fn regression_test_gh_261() { // Validation cases from https://aa.usno.navy.mil/calculated/juliandate?ID=AA&date=1607-01-25&era=AD&time=00%3A00%3A00.000&submit=Get+Date let validation_cases = &[2308028.5, 2308087.5, 2308240.5, 2308362.5]; - for (mcnt, month) in [1, 3, 8, 12].iter().enumerate() { - for day in 25..=30 { - let epoch = Epoch::from_gregorian_utc(1607, *month, day, 0, 0, 0, 0); - - // The initial validation cases is 25 days away. - let expected = validation_cases[mcnt] - 25.0 + (day as f64); - - assert_eq!( - epoch.to_jde_utc_days(), - expected, - "err = {}", - epoch.to_jde_utc_days() - expected - ); - - assert_eq!( - format!("{epoch}"), - format!("1607-{month:02}-{day:02}T00:00:00 UTC") - ); + for year in [1607, 1809, 1988, 2027, 2021] { + for (mcnt, month) in [1, 3, 8, 12].iter().enumerate() { + for day in 25..=30 { + let epoch = Epoch::from_gregorian_utc(year, *month, day, 0, 0, 0, 0); + + // Check the Julian date only for the validation cases we have. + if year == 1607 { + // The initial validation cases is 25 days away. + let expected = validation_cases[mcnt] - 25.0 + (day as f64); + + assert_eq!( + epoch.to_jde_utc_days(), + expected, + "err = {}", + epoch.to_jde_utc_days() - expected + ); + } + + // Always check the formatting of the date. + assert_eq!( + format!("{epoch}"), + format!("{year}-{month:02}-{day:02}T00:00:00 UTC") + ); + } } } } From 878b9b8d1cf8eaa83159a6f53242057e9bed6f7d Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 16:24:19 -0600 Subject: [PATCH 10/16] Uncomment tests I had commented for debugging --- tests/epoch.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/epoch.rs b/tests/epoch.rs index 49a1b34..d94c748 100644 --- a/tests/epoch.rs +++ b/tests/epoch.rs @@ -1855,14 +1855,14 @@ fn test_epoch_formatter() { let bday = Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 37); - // let fmt_iso_ord = Formatter::new(bday, ISO8601_ORDINAL); - // assert_eq!(format!("{fmt_iso_ord}"), "2000-060"); + let fmt_iso_ord = Formatter::new(bday, ISO8601_ORDINAL); + assert_eq!(format!("{fmt_iso_ord}"), "2000-060"); - // let fmt_iso_ord = Formatter::new(bday, Format::from_str("%j").unwrap()); - // assert_eq!(format!("{fmt_iso_ord}"), "060"); + let fmt_iso_ord = Formatter::new(bday, Format::from_str("%j").unwrap()); + assert_eq!(format!("{fmt_iso_ord}"), "060"); - // let fmt_iso = Formatter::new(bday, ISO8601); - // assert_eq!(format!("{fmt_iso}"), format!("{bday}")); + let fmt_iso = Formatter::new(bday, ISO8601); + assert_eq!(format!("{fmt_iso}"), format!("{bday}")); let fmt = Formatter::new(bday, ISO8601_DATE); assert_eq!(format!("{fmt}"), format!("2000-02-29")); From b18ecf64994d1a63612a660fd7d8b7927e470390 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 17:00:55 -0600 Subject: [PATCH 11/16] Trying to cleanup the compute_gregorian function --- src/epoch/gregorian.rs | 112 ++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 53 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index a0bf7d8..9258a02 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -25,20 +25,59 @@ impl Epoch { time_scale: TimeScale, ) -> (i32, u8, u8, u8, u8, u8, u32) { let duration_wrt_ref = duration + time_scale.gregorian_epoch_offset(); - let (sign, days, hours, minutes, seconds, milliseconds, microseconds, nanos) = - duration_wrt_ref.decompose(); + let sign = duration_wrt_ref.signum(); + let (days, hours, minutes, seconds, milliseconds, microseconds, nanos) = if sign < 0 { + // For negative epochs, the computation of days and time must account for the time as it'll cause the days computation to be off by one. + let (_, days, hours, minutes, seconds, milliseconds, microseconds, nanos) = + duration_wrt_ref.decompose(); - let rslt = duration_wrt_ref.to_unit(Unit::Day); - dbg!(rslt); + // Recompute the time since we count backward and not forward for negative durations. + let time = Duration::compose( + 0, + 0, + hours, + minutes, + seconds, + milliseconds, + microseconds, + nanos, + ); + + // Compute the correct time. + let (_, _, hours, minutes, seconds, milliseconds, microseconds, nanos) = + (24 * Unit::Hour - time).decompose(); + + let days_f64 = if time > Duration::ZERO { + -((days + 1) as f64) + } else { + -(days as f64) + }; - let days_f64 = if sign < 0 { - // This looks hacky but it avoids lots of edge cases. - -((days + 1) as f64) + ( + days_f64, + hours, + minutes, + seconds, + milliseconds, + microseconds, + nanos, + ) } else { - days as f64 + // For positive epochs, the computation of days and time is trivally the decomposition of the duration. + let (_, days, hours, minutes, seconds, milliseconds, microseconds, nanos) = + duration_wrt_ref.decompose(); + ( + days as f64, + hours, + minutes, + seconds, + milliseconds, + microseconds, + nanos, + ) }; - let (mut year, mut days_in_year) = div_rem_f64(days_f64, DAYS_PER_YEAR_NLD); + let (mut year, mut days_in_year) = div_rem_f64(days, DAYS_PER_YEAR_NLD); year += HIFITIME_REF_YEAR; // Base calculation was on 365 days, so we need to remove one day per leap year @@ -83,50 +122,17 @@ impl Epoch { // Directly compute the day from the computed month, and ensure that day counter is one indexed. let day = days_in_year - cumul_days[month - 1] as f64 + 1.0; - if sign == -1 { - // Our date calculation is independant from the time in that day, so we fix it here. - - // Recompute the time since we count backward and not forward for negative durations. - let time = 24 * Unit::Hour - - Duration::compose( - 0, - 0, - hours, - minutes, - seconds, - milliseconds, - microseconds, - nanos, - ); - - // Compute the correct time. - let (_, _, hours, minutes, seconds, milliseconds, microseconds, nanos) = - time.decompose(); - - ( - year, - month as u8, - day as u8, - hours as u8, - minutes as u8, - seconds as u8, - (nanos - + microseconds * NANOSECONDS_PER_MICROSECOND - + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, - ) - } else { - ( - year, - month as u8, - day as u8, - hours as u8, - minutes as u8, - seconds as u8, - (nanos - + microseconds * NANOSECONDS_PER_MICROSECOND - + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, - ) - } + ( + year, + month as u8, + day as u8, + hours as u8, + minutes as u8, + seconds as u8, + (nanos + + microseconds * NANOSECONDS_PER_MICROSECOND + + milliseconds * NANOSECONDS_PER_MILLISECOND) as u32, + ) } #[cfg(feature = "std")] From 0496fda0164c705655a4529585a24b272ebd0b48 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 17:13:32 -0600 Subject: [PATCH 12/16] Down to one failing test --- src/epoch/gregorian.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 9258a02..51df509 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -98,8 +98,9 @@ impl Epoch { days_in_year += 1.0; } } - if (days_in_year > DAYS_PER_YEAR_NLD && !is_leap_year(year)) - || (days_in_year > DAYS_PER_YEAR_NLD + 1.0 && is_leap_year(year)) + // Check for greater than or equal because the days are still zero indexed here. + if (days_in_year >= DAYS_PER_YEAR_NLD && !is_leap_year(year)) + || (days_in_year >= DAYS_PER_YEAR_NLD + 1.0 && is_leap_year(year)) { // We've overflowed the number of days in a year because of the leap years year += 1; From ddb63b4c55a33e99c6381c6816a0acc9b1496d81 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 17:13:32 -0600 Subject: [PATCH 13/16] Down to one failing test --- src/epoch/gregorian.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index 51df509..ea8e11f 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -264,13 +264,16 @@ impl Epoch { source: DurationError::Overflow, }) } - Some(days) => Unit::Day * i64::from(days), + Some(days) => { + // Initialize the duration as the number of days since the reference year (may be negative). + Unit::Day * i64::from(days) + } }, }; // Now add the leap days for all the years prior to the current year if year >= HIFITIME_REF_YEAR { - // Add days + // Add days until, but not including, current year. for y in HIFITIME_REF_YEAR..year { if is_leap_year(y) { duration_wrt_ref += Unit::Day; @@ -285,7 +288,8 @@ impl Epoch { } } - // Add the seconds for the months prior to the current month + // Add the seconds for the months prior to the current month. + // Correctly accounts for the number of days based on whether this is a leap year or not. let cumul_days = if is_leap_year(year) { CUMULATIVE_DAYS_FOR_MONTH_LEAP_YEARS } else { From 6cab4656c79334708055aaa9b9ed842a5ab13761 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 18:16:57 -0600 Subject: [PATCH 14/16] Found the remaining bug in the Gregorian computation --- src/epoch/gregorian.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/epoch/gregorian.rs b/src/epoch/gregorian.rs index ea8e11f..e940200 100644 --- a/src/epoch/gregorian.rs +++ b/src/epoch/gregorian.rs @@ -91,6 +91,10 @@ impl Epoch { // We've underflowed the number of days in a year because of the leap years year -= 1; days_in_year += DAYS_PER_YEAR_NLD; + // If we had incorrectly removed one day of the year in the previous loop, fix it here. + if is_leap_year(year) { + days_in_year += 1.0; + } } } else { for y in year..HIFITIME_REF_YEAR { From a1e084622ab610603c65e7f2a1f559c442cb70cb Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 18:23:46 -0600 Subject: [PATCH 15/16] Lint --- src/duration/kani_verif.rs | 4 ++-- src/timeunits.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/duration/kani_verif.rs b/src/duration/kani_verif.rs index 2ef9081..8895438 100644 --- a/src/duration/kani_verif.rs +++ b/src/duration/kani_verif.rs @@ -43,9 +43,9 @@ fn formal_duration_truncated_ns_reciprocity() { // Then it does not fit on a i64, so this function should return an error assert_eq!( dur_from_part.try_truncated_nanoseconds(), - Err(Err(EpochError::Duration { + Err(EpochError::Duration { source: DurationError::Overflow, - })) + }) ); } else if centuries == -1 { // If we are negative by just enough that the centuries is negative, then the truncated seconds diff --git a/src/timeunits.rs b/src/timeunits.rs index a9982df..b334b45 100644 --- a/src/timeunits.rs +++ b/src/timeunits.rs @@ -267,7 +267,7 @@ impl Mul for Unit { // Does not fit on an i64, let's do this again on an 128. let q = i128::from(q); match q.checked_mul(factor.into()) { - Some(total_ns) => Duration::from_total_nanoseconds(i128::from(total_ns)), + Some(total_ns) => Duration::from_total_nanoseconds(total_ns), None => { if q.is_negative() { Duration::MIN From 39a5d4345c7d084a09792ca3c7dd1cb1a4fa3236 Mon Sep 17 00:00:00 2001 From: Christopher Rabotin Date: Sun, 19 May 2024 18:35:45 -0600 Subject: [PATCH 16/16] Fix kani build --- src/duration/kani_verif.rs | 2 +- src/duration/mod.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/duration/kani_verif.rs b/src/duration/kani_verif.rs index 8895438..091b022 100644 --- a/src/duration/kani_verif.rs +++ b/src/duration/kani_verif.rs @@ -10,7 +10,7 @@ // Here lives all of the formal verification for Duration. -use super::{Duration, DurationError}; +use super::{Duration, DurationError, EpochError}; use crate::NANOSECONDS_PER_CENTURY; use kani::Arbitrary; diff --git a/src/duration/mod.rs b/src/duration/mod.rs index ed79a36..584ccbf 100644 --- a/src/duration/mod.rs +++ b/src/duration/mod.rs @@ -8,10 +8,8 @@ * Documentation: https://nyxspace.com/ */ -use crate::errors::DurationError; -use crate::{ - EpochError, SECONDS_PER_CENTURY, SECONDS_PER_DAY, SECONDS_PER_HOUR, SECONDS_PER_MINUTE, -}; +use crate::errors::{DurationError, EpochError}; +use crate::{SECONDS_PER_CENTURY, SECONDS_PER_DAY, SECONDS_PER_HOUR, SECONDS_PER_MINUTE}; pub use crate::{Freq, Frequencies, TimeUnits, Unit};