diffInMonths inconsistent, adding a day makes the difference bigger instead of smaller

[imerr]

Hello,

I encountered an issue with the following code:

$start = new Carbon("2022-11-11 22:29:50.000000");
$end = $start->copy()->addMonths(3);
$now = $start->copy()->addDay();

echo "diffInMonths\n";
echo $start->diffInMonths($end) . "\n";
echo $now->diffInMonths($end) . "\n";

echo "\nfloatDiffInMonths\n";
echo $start->floatDiffInMonths($end) . "\n";
echo $now->floatDiffInMonths($end) . "\n";

echo "\nfloatDiffInRealMonths\n";
echo $start->floatDiffInRealMonths($end) . "\n";
echo $now->floatDiffInRealMonths($end) . "\n";

Carbon version: 2.x/3.x

PHP version: 7.4.28 / Whatever https://try-carbon.herokuapp.com/ uses

I expected to get:

diffInMonths
3
2

floatDiffInMonths
3
2.9<something>

floatDiffInRealMonths
3
2.9<something>

But I actually get:

diffInMonths
3
2

floatDiffInMonths
3
3.0055439548131

floatDiffInRealMonths
3
3.0055439548131

Thanks!

I have seen and read #2264

The "counting every day by fraction" way from #2264 (comment) results in:

>>> CarbonHelper::monthDifferenceDayByDay($start, $end)
=> 3.0595238095238
>>> CarbonHelper::monthDifferenceDayByDay($now, $end)
=> 3.0261904761905

Which is better - at least adding a day makes the difference smaller now, but I'd still expect 3 and < 3 as the results

[imerr]

I've thrown together this, which seems to produce more consistent results (not quite sure if youd call them correct?)

    /**
     * Adds $months months to the date with no overflow while attempting to keep the calendar day/hour/minute/second the same (or lower)
     * @param Carbon $carbon Is copied, not modified
     * @param $day
     * @return Carbon
     */
    public static function addMonthsKeepDay(Carbon $carbon, int $months, int $day): Carbon {
        $date = $carbon->copy()->addMonthsNoOverflow($months)->setUnitNoOverflow('day', $day, 'month');
        return $carbon->copy()->setDate($date->year, $date->month, $date->day);
    }

    /**
     * Returns the difference in months between the two dates
     * This works around an inconsistency in carbon base code
     * @param Carbon $start
     * @param Carbon $end
     * @return float
     */
    public static function correctMonthDifference(Carbon $start, Carbon $end) {
        // the built-in carbon difference produces some weird results
        // see https://github.com/briannesbitt/Carbon/issues/2604
        // so we have a custom way to produce more consistent output for our needs

        // swap start is greater
        if ($start->gt($end)) {
            [$start, $end] = [$end, $start];
        }

        $diff = 0;

        // start by counting up whole months until its we reach or overshoot end
        $current = $start->copy();
        do {
            $prevMonth = $current;
            $current = static::addMonthsKeepDay($current, 1, $start->day);
            $diff += 1;
        } while ($current->lt($end));

        if ($current->gt($end)) {
            // if we overshot $end, we need to subtract a partial month
            // in order to do that we take how many days we overshot by
            $dayDifferenceToEnd = $current->floatDiffInDays($end);
            // and divide that by the amount of days in this "month segment"
            // for example:
            // - for 11.01.22 to 11.02.22 thats 31 days
            // - for 11.02.22 to 11.03.22 thats 28 days
            $daysToPrev = $prevMonth->floatDiffInDays($current);
            $diff -= $dayDifferenceToEnd / $daysToPrev;
        }

        return $diff;
    }

Which passes the following tests:

    public function test_month_diff() {
        $data = [
            ["2022-11-11 22:29:50.000000", "2023-02-11 22:29:50.000000", 3],
            ["2022-11-12 22:29:50.000000", "2023-02-11 22:29:50.000000", 3 - 1 / 31],
            ["2022-11-10 22:29:50.000000", "2023-02-11 22:29:50.000000", 3 + 1 / 28],
            ["2022-11-9 22:29:50.000000", "2023-02-11 22:29:50.000000", 3 + 2 / 28],
            // test feb overflow
            ["2022-11-30 22:29:50.000000", "2023-02-28 22:29:50.000000", 3],
            ["2022-11-30 22:29:50.000000", "2023-03-01 22:29:50.000000", 3 + 1 / 30],
            ["2022-12-31 22:29:50.000000", "2023-03-01 22:29:50.000000", 2 + 1 / 31],
            ["2022-12-30 22:29:50.000000", "2023-03-01 22:29:50.000000", 2 + 1 / 30],
        ];
        foreach ($data as [$start, $end, $expected]) {
            $this->assertEqualsWithDelta($expected, CarbonHelper::correctMonthDifference(new Carbon($start), new Carbon($end)), 0.00001, $start . " - " . $end);
        }
    }

    public function test_month_diff_smaller() {
        $start = new Carbon("2022-01-01 02:29:50.000000");
        $end = new Carbon("2025-11-11 02:29:50.000000");
        $prevDif = CarbonHelper::correctMonthDifference($start, $end);
        while ($start->lt($end)) {
            $start->addRealDay();
            $dif = CarbonHelper::correctMonthDifference($start, $end);
            $this->assertLessThan($prevDif, $dif, $start);
            $prevDif = $dif;
        }
        $this->assertEquals(0, $prevDif, $start);
    }
    
    public function test_month_diff_larger() {
        $start = new Carbon("2022-01-01 02:29:50.000000");
        $end = $start->copy();
        $until = new Carbon("2026-01-01 02:29:50.000000");
        $prevDif = CarbonHelper::correctMonthDifference($start, $end);
        $this->assertEquals(0, $prevDif, $end);
        while ($end->lt($until)) {
            $end->addRealDay();
            $dif = CarbonHelper::correctMonthDifference($start, $end);
            $this->assertGreaterThanOrEqual($prevDif, $dif, $end);
            $prevDif = $dif;
        }
        $this->assertEquals(4 * 12, $prevDif, $end);
    }
    ```

[kylekatarnls]

Actually I doubt we could improve this without degrading other cases.

For instance, I tried to add the following difference in test_month_diff: "2022-01-29 22:29:50.000000", "2022-02-28 22:29:50.000000"

The result is 1. Right now, and so when end = 2022-02-28, then starts 2022-01-28, 2022-01-29 , 2022-01-30 and 2022-01-31 will all return a difference of 1 which could also lead to quite bad surprises.

Also if I compare side-by-side 02-28 case with your 02-11 it should be 1 - 1 / 31, right:

["2022-11-12 22:29:50.000000", "2023-02-11 22:29:50.000000", 3 - 1 / 31],
["2022-01-29 22:29:50.000000", "2022-02-28 22:29:50.000000", 1 - 1 / 31],

I mean if we make the algo evolve, we need some explanation to provide about why 01-29 to 02-28 is different from 01-12 to 02-11.

Also about why sometimes it will be 1/31 when none of the start date nor the end date have 31 days. How do we choose the denominator of the fraction?

The more I dig in this, the smaller are my hope to find something that could be continuous (e.g. adding time to start always reduces the result, adding time to end always increases the results and reciprocally) and match with intuitive result that interval between Nth and Nth of a other month should be an integer result.

This last point is actually the one that is commonly agreed by humans but has no reason in mathematics because January 16th 00:00 to February 16th 00:00 interval is actually containing more than half of January and more than half of February (e.g. 16/31 of January + 15/28 of February) it's actually 1.052 (this result is the more correct for maths).

[imerr]

Actually I doubt we could improve this without degrading other cases.
For instance, I tried to add the following difference in test_month_diff: "2022-01-29 22:29:50.000000", "2022-02-28 22:29:50.000000"
The result is 1. Right now, and so when end = 2022-02-28, then starts 2022-01-28, 2022-01-29 , 2022-01-30 and 2022-01-31 will all return a difference of 1 which could also lead to quite bad surprises.

Yeah, this isn't an easy thing to solve and the exact behaviour people want will likely depend on what you're trying to do.
I'm currently building something along the lines of a subscription service that charges monthly, you've got the option to change your plan during the month and it'll give you a discount based on how far the month is gone (and of course that shouldn't be more expensive :D)
I guess the other way to solve this would be discount billing until the start of the month and always use the 1st as the billing date, but thats not as end-user friendly (they might want to be charged on different days as a start)

I mostly provided the code I threw together as a starting point/to have something to talk about, its definitely not ready for shipping as a general purpose solution

Also about why sometimes it will be 1/31 when none of the start date nor the end date have 31 days. How do we choose the denominator of the fraction?

The code currently overshoots by a month and then subtracts the remaining days, guess not overshooting and adding onto the full months would make that make more sense?

["2022-11-12 22:29:50.000000", "2023-02-11 22:29:50.000000", 3 - 1 / 31],

2022-11-12 + 1 month = 2022-12-12 -> 1 months
2022-12-12 + 1 month = 2023-01-12 -> 2 months
2023-01-12 + 1 month = 2022-02-12 -> 3 months, 1 day too far!
2023-02-12 - 2023-01-12 = 31 days -> result is 3 - 1 (day too far) / 31 (days to date when we didnt overshoot yet)

["2022-01-29 22:29:50.000000", "2022-02-28 22:29:50.000000", 1 - 1 / 31],

2022-01-29 + 1 month (no overflow, so capped to 28 days this feb) = 2022-02-28 -> 1 month

The whole capping the month thing makes sense for my use case, but is slightly odd in general. I agree

Thanks for looking into this 👍

[tudor-gala]

With Carbon 3 dropping the old diffInMonths() functionality, this is really an issue as we cannot reliably convert the float value to an integer because of this bug.

The old diffInMonths() worked much better IMHO, why is it no longer an option?

[kylekatarnls]

It's explained in the doc at the v3 migration section and more in detail in #2119

How float vs. integer can it be an issue? You can just use (int), floor(), round(), or whatever way you prefer to truncate the result ((int) would be what is equivalent to v2) so the difference is that instead of it to happen in a black box, you're in control and can decide how it makes sense in your application to get the integer result, from the actual value which by nature does not make sense to be integer.

worked much better

The huge list of issues opened in the past about the v2 behavior tells otherwise. The point is that when it comes to diff in months from stuff like: from February 15th 12:40 to March 14th 13:51, then you start slightly moving the start or the end of the range or even more tricky: working with different timezone for the end and the start, you get a lot of different expectations depending on the business case. There is no 1 single behavior everyone will be happy with, some are crafting subscription renewals, some are doing progress bar for days series, planning, calendars, etc. That's the reason why the default behavior should not be too opiniated but let some control to the caller.

Note that the result also changes slightly depending on which version of PHP you use, so if you find a precise case in v3 you think is always wrong whatever would be the context, you should open an issue with this exact case, how to reproduce, the result you get and the one you expect.

why is it no longer an option?

Same for that, default behavior changed and giving float is more precise, so AFAIK v3 did not "remove" an option, if you find yourself unable to achieve a precise thing that you were able in v2, you should provide your v2 code for inspection, so to check what would be the good way in v3.