bugGNU Octave - Bugs: bug #60671, Wrong result when subtracting one...

 
 

bug #60671: Wrong result when subtracting one month from 31-MON-YEAR date

Submitter:  Georg Wiora <gwiora>
Submitted:  Tue 25 May 2021 03:29:08 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  gwiora Open/Closed:  * Closed
Release:  * 7.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 17 Oct 2022 07:01:54 AM UTC, comment #13: 

Thanks for providing and for applying the changes.

The new tests are passing in our CI.

Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Wed 12 Oct 2022 12:56:32 PM UTC, comment #12: 

sorry this sat so long.

looking it over, the patch still applies cleanly once I change the target NEWS file item. all tests still pass.

this is a fairly straightforward bugfix.  made the fix note in the 7.3.0 news file and pushed to stable as:
https://hg.savannah.gnu.org/hgweb/octave/rev/b8afc6e7d9b4

Nicholas Jankowski <nrjank>
Group Member
Mon 10 Oct 2022 07:40:03 AM UTC, comment #11: 

This is still not corrected in octave 7.2.0. Does anyone want to include this fix?

Georg Wiora <gwiora>
Mon 08 Nov 2021 08:43:24 AM UTC, comment #10: 

Thank you very much for considering my patch! I don't insist on having my mail address added to the code base.

I am not deep enough in octave development to actively contribute patches. I am happy if I can help from time to time.

Georg Wiora <gwiora>
Fri 05 Nov 2021 06:02:28 PM UTC, comment #9: 

ok, thanks for the reminder Georg.

it helps maintainers consider code changes when they've been made into a patch on the Octave codebase. (See https://wiki.octave.org/Mercurial if you want to see how to produce one.)

I had difficulty creating a patch by just adding your file to the current codebase, it appears you based it on a fairly old version of the function with a different copyright header, and your system used a different line ending (linux usually user LF, Windows uses CR/LF, yours just used CR.). Octave should be able to be set to save with LF endings.  for the former, always try to make changes using the latest development sources (see link above).

Anyway, adjusting for those was fairly simple. i also added a few of the examples in these comments as tests and checked them against matlab.  The patched file passes all tests and seems to fix this bug.

attached is the patch that makes these changes and adds a comment to NEWS about the bugfix as well as the updated addtodate.m file with those changes

i used your name as patch author, but do you want an email address added as well? that can be added if/when this gets pushed to the codebase.

(file #52203, file #52204)

Nicholas Jankowski <nrjank>
Group Member
Fri 05 Nov 2021 05:51:40 PM UTC, comment #8: 

reviewing status now.

Nicholas Jankowski <nrjank>
Group Member
Fri 05 Nov 2021 10:22:29 AM UTC, comment #7: 

Would be happy if someone could include this fix into next release.

Georg Wiora <gwiora>
Wed 26 May 2021 12:16:12 PM UTC, comment #6: 

I entered the following test cases in Matlab onramp:


>> datestr(addtodate(datenum('2021-02-28'),1,'month'))

ans = 28-Mar-2021

>> datestr(addtodate(datenum('2021-03-31'),-1,'month'))

ans = 28-Feb-2021


This is the same behavior that my fix (file #51475) provides.

Georg Wiora <gwiora>
Tue 25 May 2021 04:14:34 PM UTC, comment #5: 

For the records: Bug is still present in octave 6.1.0 (Mac OSX).

Georg Wiora <gwiora>
Tue 25 May 2021 04:10:37 PM UTC, comment #4: 

I don't see the necessity of loops here. eomday does allready calculate the last of month. Could you please post your fix as a complete file?

Here is the excerpt from my fix (see full file in attachments)


      ## adjust the years and months if the date rolls over a year
      dtmp(:,1) += floor ((dtmp(:,2)-1)/12);
      dtmp(:,2) = mod (dtmp(:,2)-1, 12) + 1;
      # BUGFIX gwiora: Avoid days beyond end of month
      beyondEOM = dtmp(:,3)>eomday(dtmp(:,1),dtmp(:,2));
      dtmp(beyondEOM,3) = eomday(dtmp(beyondEOM,1),dtmp(beyondEOM,2));


Georg Wiora <gwiora>
Tue 25 May 2021 03:56:46 PM UTC, comment #3: 

I think changing changing lines 65 and 66 in addtodate.m to the following loops will be safer. Is this OK for edge cases?


do
    ii = (dtmp(:,2) < 1); # these entries are underflowing
    dtmp(ii,2) += 12;  # go from Month 0 to Month 12, etc
    dtmp(ii,1) -= 1;   # and decrement year
until (~any(ii));

do
    ii = (dtmp(:,2) > 12); # these entries are overflowing
    dtmp(ii,2) -= 12;  # go from Month 13 to Month 1, etc
    dtmp(ii,1) += 1;   # and increment year
until (~any(ii));



Anonymous
Tue 25 May 2021 03:53:02 PM UTC, comment #2: 

I added a file with a suggested fix, but I am not shure if this is allways the intended behaviour. Also I do not know the matlab behaviour, but Matlab compatibility should be provided. My fix uses EOMday() function to check if the resulting day is beyond end of previous month. This leads to the situation that some operations with different dates may produce the same result date, but at least the result is allways in the previous month:

>> datestr(addtodate(datenum('31-Dec-2020'),-1,'month'))
ans = 30-Nov-2020
>> datestr(addtodate(datenum('30-Dec-2020'),-1,'month'))
ans = 30-Nov-2020
>> datestr(addtodate(datenum('29-Dec-2020'),-1,'month'))
ans = 29-Nov-2020


Matlab manual states about adding negative numbers:

Adding a negative quantity to the indicated date field rolls back the calender on the indicated field. If the addition causes the field to roll back, MATLAB adjusts the next less significant fields accordingly.

This does not make absolutely clear what happens when adding negative numbers.
Can someone prove the Matlab behaviour with these test cases?

TODO: Add discussed situations as test cases to bug fixed file.

Georg Wiora <gwiora>
Tue 25 May 2021 03:50:26 PM UTC, comment #1: 

Thank you for the bug report. It looks like the behavior is chaotic about whether it returns the same month or the previous month:


octave:12> datestr (addtodate (datenum('31-Jan-2020'), -1,'month'))
ans = 31-Dec-2019
octave:13> datestr (addtodate (datenum('31-Mar-2020'), -1,'month'))
ans = 02-Mar-2020
octave:14> datestr (addtodate (datenum('31-May-2020'), -1,'month'))
ans = 01-May-2020
octave:15> datestr (addtodate (datenum('31-Jul-2020'), -1,'month'))
ans = 01-Jul-2020
octave:16> datestr (addtodate (datenum('31-Aug-2020'), -1,'month'))
ans = 31-Jul-2020
octave:17> datestr (addtodate (datenum('31-Oct-2020'), -1,'month'))
ans = 01-Oct-2020
octave:18> datestr (addtodate (datenum('31-Dec-2020'), -1,'month'))
ans = 01-Dec-2020


For the other months, it at least seems to be consistently decrementing the month:


octave:20> datestr(addtodate (datenum('29-Feb-2020'), -1,'month'))
ans = 29-Jan-2020
octave:21> datestr(addtodate (datenum('30-Apr-2020'), -1,'month'))
ans = 30-Mar-2020
octave:22> datestr(addtodate (datenum('30-Jun-2020'), -1,'month'))
ans = 30-May-2020
octave:23> datestr(addtodate (datenum('30-Sep-2020'), -1,'month'))
ans = 30-Aug-2020
octave:24> datestr(addtodate (datenum('30-Nov-2020'), -1,'month'))
ans = 30-Oct-2020


The bug seems to be in these two lines within addtodate.m, which fail sometimes when the second argument q is negative:


      dtmp(:,1) += floor ((dtmp(:,2)-1)/12);
      dtmp(:,2) = mod (dtmp(:,2)-1, 12) + 1;


Anonymous
Tue 25 May 2021 03:29:08 PM UTC, original submission:  

Subtracting one month from an end of month date in a month with 31 days returns first of the same month as result instead of last day of earlier month.

Examples:


>> datestr(addtodate(datenum('31-DEC-2020'),-1,'month'))
ans = 01-Dec-2020
>> datestr(addtodate(datenum('30-NOV-2020'),-1,'month'))
ans = 30-Oct-2020
>> datestr(addtodate(datenum('31-OCT-2020'),-1,'month'))
ans = 01-Oct-2020


The first and last result is expected to be 30-Nov-2020 and 30-Sep-2020.

I tested this on Mac OSX 10.15.7 with octave 5.2.0. The addtodate script was located at:


/Applications/Octave-5.2.0.app/Contents/Resources/usr/Cellar/octave-octave-app@5.2.0/5.2.0/share/octave/5.2.0/m/time/addtodate.m


Georg Wiora <gwiora>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #52203:  addtodate_endofmonthfix_bug60671.diff added by nrjank (3KiB - application/octet-stream - patch and updated function file)
file #52204:  addtodate.m added by nrjank (5KiB - text/plain - patch and updated function file)
file #51475:  addtodate.m added by gwiora (4KiB - application/octet-stream - Bug fixed version using EOMDate)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by gwiora (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-10-17 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-10-12 nrjank StatusPatch Submitted Ready For Test
    2022-10-12 nrjank Release5.2.0 7.2.0
    2021-11-05 nrjank Attached File- Added addtodate_endofmonthfix_bug60671.diff, #52203
        Attached File- Added addtodate.m, #52204
        StatusNone Patch Submitted
    2021-05-25 gwiora Attached File- Added addtodate.m, #51475

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code