bugGNU Octave - Bugs: bug #50508, datenum() crashes for non-integer...

 
 

bug #50508: datenum() crashes for non-integer month arrays

Submitter:  Lars Kindermann <larskindermann>
Submitted:  Fri 10 Mar 2017 05:44:33 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 16 Mar 2017 04:17:43 PM UTC, comment #14: 

I pushed the patch under your name to the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/92d60bf45889).  I added some BIST tests for fractional years as well.

I'm going to close this report, finally!

Rik <rik5>
Group administrator
Thu 16 Mar 2017 04:07:52 AM UTC, comment #13: 

I attached a patch that hopefully handles fractional years correctly now. I left the current code untouched for normal years and have added some lines which are called just in case of fractional years.


   ## Lookup number of days since start of the current year.
   day += monthstart(mod (month-1,12) + 1) + 60;

+  ## Treat fractional years, by converting the fraction to days
+  if (any (year != fix (year)))
+    fracyear = year - floor (year);
+    year = floor (year);
+    day += fracyear .* (365 + is_leap_year (year+1));
+  endif
+
   ## Add number of days to the start of the current year.  Correct
   ## for leap year every 4 years except centuries not divisible by 400.


Btw: It is not a good idea to test datenum using datestr as datestr calls datenum itself. Better use datevec.


datevec (datenum (2000.99999, 1:14, 1))
ans =

   2000.0000     12.0000     31.0000     23.0000     54.0000     43.7760
   2001.0000      1.0000     31.0000     23.0000     54.0000     43.7760
   2001.0000      2.0000     28.0000     23.0000     54.0000     44.6400
   2001.0000      3.0000     31.0000     23.0000     54.0000     44.6400
   2001.0000      4.0000     30.0000     23.0000     54.0000     44.6400
   2001.0000      5.0000     31.0000     23.0000     54.0000     44.6400
   2001.0000      6.0000     30.0000     23.0000     54.0000     44.6400
   2001.0000      7.0000     31.0000     23.0000     54.0000     44.6400
   2001.0000      8.0000     31.0000     23.0000     54.0000     44.6400
   2001.0000      9.0000     30.0000     23.0000     54.0000     44.6400
   2001.0000     10.0000     31.0000     23.0000     54.0000     44.6400
   2001.0000     11.0000     30.0000     23.0000     54.0000     44.6400
   2001.0000     12.0000     31.0000     23.0000     54.0000     44.6400
   2002.0000      1.0000     31.0000     23.0000     54.0000     44.6400


(file #40011)

Lars Kindermann <larskindermann>
Wed 15 Mar 2017 11:44:30 PM UTC, comment #12: 

The problem with fractional years was present in 4.2.1 so at least we haven't introduced anything new with our changes.  I'm sure this is related to some odd shifting of the start of the year.  The code is


  ## Set start of year to March by moving Jan. and Feb. to previous year.
  ## Correct for months > 12 by moving to subsequent years.
  year += ceil ((month-14)/12);

  ## Lookup number of days since start of the current year.
  day += monthstart(mod (month-1,12) + 1) + 60;

  ## Add number of days to the start of the current year.  Correct
  ## for leap year every 4 years except centuries not divisible by 400.
  day += 365*year + floor (year/4) - floor (year/100) + floor (year/400);


Whatever we are doing, I would expect it to cause problems for Jan. and Feb.  Indeed, that is what this code shows.


datestr (datenum (2000.99999, 1:14, 1))
ans =

30-Dec-2000 23:54:44
30-Jan-2001 23:54:44
28-Feb-2001 23:54:44
31-Mar-2001 23:54:44
30-Apr-2001 23:54:44
31-May-2001 23:54:44
30-Jun-2001 23:54:44
31-Jul-2001 23:54:44
31-Aug-2001 23:54:44
30-Sep-2001 23:54:44
31-Oct-2001 23:54:44
30-Nov-2001 23:54:44
31-Dec-2001 23:54:44
31-Jan-2002 23:54:44

datestr (datenum (2001.99999, 1:14, 1))
ans =

31-Dec-2001 23:54:44
31-Jan-2002 23:54:44
28-Feb-2002 23:54:44
31-Mar-2002 23:54:44
30-Apr-2002 23:54:44
31-May-2002 23:54:44
30-Jun-2002 23:54:44
31-Jul-2002 23:54:44
31-Aug-2002 23:54:44
30-Sep-2002 23:54:44
31-Oct-2002 23:54:44
30-Nov-2002 23:54:44
31-Dec-2002 23:54:44
31-Jan-2003 23:54:44


@Lars: Do you want to take a look at this one?  It looks to me like in the lookup for number of days since the start of the current year the constant +60 needs to be +61 for leap years.

Rik <rik5>
Group administrator
Wed 15 Mar 2017 11:33:29 PM UTC, comment #11: 

I pushed the change for fractional months and the syntax


datenum (YEAR, MONTH, DAY)


See http://hg.savannah.gnu.org/hgweb/octave/rev/a21f8372bb76.



Rik <rik5>
Group administrator
Wed 15 Mar 2017 11:21:30 PM UTC, comment #10: 

Sorry, there is still a glitch in datenum(), this time with fractional years:


datestr(datenum(2000.5:2004.5,1,1))
ans =

01-Jul-2000 12:00:00
02-Jul-2001 12:00:00
02-Jul-2002 12:00:00
02-Jul-2003 12:00:00
01-Jul-2004 12:00:00


or, better visible here:


datestr(datenum(2000.999999:2004.999999,1,1))
ans =

30-Dec-2000 23:59:28
31-Dec-2001 23:59:28
31-Dec-2002 23:59:28
31-Dec-2003 23:59:28
30-Dec-2004 23:59:28


The number of days in a year used in the calculation seems to ignore leap years.

Lars Kindermann <larskindermann>
Wed 15 Mar 2017 10:57:40 PM UTC, comment #9: 

Yes, datestr() seems to use only the first entry in an array to auto-select the format. If time there is zero, all other rows also display the date only. I was fooled by that. Perhaps that should be changed in datestr(). datenum() works fine for me now.

Lars Kindermann <larskindermann>
Wed 15 Mar 2017 08:38:55 PM UTC, comment #8: 

I think everything is working correctly.  For the last example, it appears that datestr has chosen a different output format because the hour field is precisely aligned at 00 and 12.  If I force datestr to use a long format (format 0) then this is what I get.


datestr(datenum(2000:2004,2.5,1, 12,0,0), 0)
ans =

16-Feb-2000 00:00:00
15-Feb-2001 12:00:00
15-Feb-2002 12:00:00
15-Feb-2003 12:00:00
16-Feb-2004 00:00:00



Rik <rik5>
Group administrator
Wed 15 Mar 2017 01:58:40 PM UTC, comment #7: 

this works fine now:


>> datestr(datenum(2000:2004,2.5,1, 0,0,0))
ans =

15-Feb-2000 12:00:00
15-Feb-2001 00:00:00
15-Feb-2002 00:00:00
15-Feb-2003 00:00:00
15-Feb-2004 12:00:00


add one hour


>> datestr(datenum(2000:2004,2.5,1, 1,0,0))
ans =

15-Feb-2000 13:00:00
15-Feb-2001 01:00:00
15-Feb-2002 01:00:00
15-Feb-2003 01:00:00
15-Feb-2004 13:00:00


that looks good, too, same with 11 hours


>> datestr(datenum(2000:2004,2.5,1, 11,0,0))
ans =

15-Feb-2000 23:00:00
15-Feb-2001 11:00:00
15-Feb-2002 11:00:00
15-Feb-2003 11:00:00
15-Feb-2004 23:00:00


but what happens with 12 hours on non leap years?


>> datestr(datenum(2000:2004,2.5,1, 12,0,0))
ans =

16-Feb-2000
15-Feb-2001
15-Feb-2002
15-Feb-2003
16-Feb-2004


I slowly understand why matlab people decided to not support fractional month but just round it down...

Lars Kindermann <larskindermann>
Tue 14 Mar 2017 08:15:09 PM UTC, comment #6: 

I'm attaching a proposed changeset.  Please verify that it works for the corner cases you have in mind.

(file #40005)

Rik <rik5>
Group administrator
Mon 13 Mar 2017 09:19:02 PM UTC, comment #5: 

@Lars: Let me try.  I know exactly where the code is that I want to change.

Rik <rik5>
Group administrator
Mon 13 Mar 2017 08:58:42 PM UTC, comment #4: 

@Rik Are you already working on it or shall I give it a try?

Lars Kindermann <larskindermann>
Mon 13 Mar 2017 03:14:42 PM UTC, comment #3: 

Confirmed.  The part of the code that exands all inputs to common size occurs after the fractional month calculation.  The program flow should probably be:

1) Validate inputs
2) Convert to common format (strings->datevec) and common size
3) Perform datenum calculation

Rik <rik5>
Group administrator
Mon 13 Mar 2017 11:31:00 AM UTC, comment #2: 

datenum() still crashes for fractional month using the other calling convention: datenum(year,month,day):

+verbose+

>> datestr(datenum(2000:2003,2,1))

ans =

01-Feb-2000
01-Feb-2001
01-Feb-2002
01-Feb-2003
-verbose-

works, but

+verbose+

>> datestr(datenum(2000:2004,2.5,1))

error: fracmonth(4): out of bound 1
error: called from
    datenum at line 135 column 14
-verbose-

Lars Kindermann <larskindermann>
Sun 12 Mar 2017 06:30:09 PM UTC, comment #1: 

I decided to fix this on the stable release since Octave shouldn't be generating errors for legal inputs.  I checked in a changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/afa48a26e684) that adds support for fractional months and some specific tests for fractional months and leap years.

Rik <rik5>
Group administrator
Fri 10 Mar 2017 05:44:33 PM UTC, original submission:  

datenum() allows integer and fractional month inputs:


>> datestr(datenum([2017 1 1]))
ans = 01-Jan-2017

>> datestr(datenum([2017 1.5 1]))
ans = 16-Jan-2017 12:00:00


However, if fed with an array of dates, it crashes when any month value is not integer:


>> datestr(datenum([2017 1 1; 2017 1 1]))
ans =

01-Jan-2017
01-Jan-2017


>> datestr(datenum([2017 1 1; 2017 1.5 1]))
error: monthstart(1.5): subscripts must be either integers 1 to (2^63)-1 or logicals
error: called from
    datenum at line 154 column 9


>> datestr(datenum([2017 1.5 1; 2017 1.5 1]))
error: datenum: operator *: nonconformant arguments (op1 is 2x1, op2 is 2x1)
error: called from
    datenum at line 137 column 11


This patch solves the problem:


diff -r 10a22466008e scripts/time/datenum.m
--- a/scripts/time/datenum.m        Fri Mar 10 13:15:23 2017 +0100
+++ b/scripts/time/datenum.m        Fri Mar 10 18:34:27 2017 +0100
@@ -126,7 +126,7 @@
   month(month < 1) = 1;  # For compatibility.  Otherwise allow negative months.

   ## Treat fractional months, by converting the fraction to days
-  if (floor (month) != month)
+  if any(floor (month) != month)
     fracmonth = month - floor (month);
     month = floor (month);
     if ((mod (month-1,12) + 1) == 2
@@ -134,7 +134,7 @@
       ## leap year
       day += fracmonth * 29;
     else
-      day += fracmonth * monthlength ((mod (month-1,12) + 1));
+      day += fracmonth .* monthlength ((mod (month-1,12) + 1));
     endif
   endif


Remark: there is some incompatibility with matlab which simply trunctates fractional month while octave adds respective days. See Bug #50493 comment #4 for details.

Lars Kindermann <larskindermann>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #40011:  datenumv3.diff added by larskindermann (742B - text/x-patch)
file #40005:  dnum.cset added by rik5 (3KiB - application/octet-stream)
file #39965:  datenum.diff added by larskindermann (734B - text/x-patch - patch for datenum())

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by larskindermann (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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-03-16 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-03-16 larskindermann Attached File- Added datenumv3.diff, #40011
    2017-03-14 rik5 Attached File- Added dnum.cset, #40005
        StatusConfirmed Patch Submitted
    2017-03-13 rik5 StatusFixed Confirmed
        Open/ClosedClosed Open
    2017-03-12 rik5 StatusNone Fixed
        Open/ClosedOpen Closed
        Releasedev 4.2.1
    2017-03-10 larskindermann Attached File- Added datenum.diff, #39965

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code