gh-161746: Use the Neri–Schneider algorithm for ordinal-to-date conversion in datetime - #161747

I don't like these magic numbers, can you please benchmark the simpler approach (compilers are pretty smart, and should be able to deal with these), like you implemented in Python? If we accept this (I still have to think about this), they should be as close as possible for maintenance purposes.

Ok. I rewrote it to follow _ord2ymd() step by step, with plain / and % by constants. gcc 14 and clang 19 turn them into multiplications anyway (no div instructions at -O3).

Results, C implementation, µs per 1000 operations (pyperf, gcc 14.2, -O3, no PGO):

Simple vs. magic numbers: 4 of 6 benchmarks not significant, geometric mean 1.03x slower. On its own, the function takes about 6.5 ns instead of 5.5 ns (the old code: about 17 ns). That isn't worth the magic numbers, so I've pushed the simple version.

It still matches the old code for every positive int ordinal (gcc and clang) and for all dates in the supported range.

Add a link, and a rough % speedup.

Done.

This might actually be a good opportunity to use a property test for the property that theclass.fromordinal(d.toordinal()) == d (handwavy)

Done. I replaced my loop with a property test: fromordinal(d.toordinal()) == d for strategies.dates(). The ends of the 4-, 100- and 400-year cycles, and min/max date, are added as @example, so they run also without Hypothesis. The date is created again with self.theclass, so the test checks the class under test.

I believe you can do divmod(4 * day_of_year + 3, 153), right?

I think the factor must stay 5: one 153-day block has 5 months. With divmod(4 * day_of_year + 3, 153), March 1 (day_of_year == 0) gives month 0, and ordinal 1 gives (0, 8, 1).

461 is 3 * 153 + 2, so this is the same:

month, rest = divmod(5 * day_of_year + 2, 153)
month += 3
day = rest // 5 + 1

About the comments: I rewrote them in both versions. Now they explain each step: why the year starts on March 1, where 4*n + 3 comes from, and what 153 and 461 mean. I removed "Figure 12" (in the paper it is the multiply-shift version, not this one) and use the DOI.

I think we can drop the 64-bit arithmetic here entirely. As far as I can tell, the only reason for it is so that date.fromordinal() with an ordinal past MAXORDINAL still gets to the same "year must be in 1..9999, not N" error that it raises on main, because fromordinal only checks ordinal < 1 before calling ord_to_ymd. The other two callers never pass anything larger than MAXORDINAL + 2: normalize_date already range-checks the ordinal, and iso_to_ymd can reach at most 10000-01-02, the last day of ISO year 9999, which its caller then rejects.

Since the calendar repeats exactly every 400 years, fromordinal can handle the error case by shifting the ordinal into the first 400-year cycle and adding the years back afterwards, which keeps the error message identical without needing a 64-bit path in the hot function:

    if (ordinal < 1) {
        PyErr_SetString(PyExc_ValueError, "ordinal must be >= 1");
        return NULL;
    }
    if (ordinal <= MAXORDINAL) {
        ord_to_ymd(ordinal, &year, &month, &day);
    }
    else {
        /* Out of range: find the year anyway, so new_date_subclass_ex()
         * raises the usual "year must be in 1..9999, not N" error.  The
         * calendar repeats every 400 years (DI400Y days), so convert the
         * equivalent date in the first 400-year cycle and add the years
         * back. */
        int cycles = (ordinal - 1) / DI400Y;
        ord_to_ymd(ordinal - cycles * DI400Y, &year, &month, &day);
        year += 400 * cycles;
    }
    return new_date_subclass_ex(year, month, day, type);

With that change, every intermediate value in ord_to_ymd fits in a uint32_t — 4 * days + 3 tops out around 14.6 million — so the function can assert ordinal <= MAXORDINAL + 2 and the comment explaining why the century step is done in 64 bits can go away. I checked this version against the ord_to_ymd on main for every positive int, and the year, month and day all match.

I did not see a measurable speed difference on x86-64 with either gcc or clang, so this is mostly an argument for simplicity. It may help somewhat on 32-bit platforms, where the 64-bit division can turn into a library call, though I have not measured that. The pure Python version does not need an equivalent change, since it uses arbitrary-precision integers and floor division.

Done, I used your version. ord_to_ymd() is now uint32_t only, with assert(ordinal <= MAXORDINAL + 2), and fromordinal() handles bigger ordinals with the 400-year shift. I checked it against main for all positive int, and the error message is the same. I also added a test that the error still has the correct year for some ordinals after MAXORDINAL.

I checked the changes twice.