bugGNU Octave - Bugs: bug #41461, cast to integer type incorrect

 
 

bug #41461: cast to integer type incorrect

Submitted by:  Mike Sander <msander>
Submitted on:  Wed 05 Feb 2014 02:44:26 AM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: NoneAssigned to: None
Originator Name: mike sanderOpen/Closed: Open
Release: devOperating System: Any

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 07 Feb 2014 09:27:06 PM UTC, comment #8:

Could someone from the octave community please clarify what are the requirement for cast of integer values to char?

my goal is to finish this patch, get it right and have it accepted/applied. presently, i am unclear what the requirements are.

Mike Sander <msander>
Fri 07 Feb 2014 09:16:32 PM UTC, comment #7:

response to comment #6.

i do follow your point "Before making this another permanent Matlab-incompatibility,...". As far as i can determine octaves current treatment of casting integer type to char is NOT compatible with matlab. I believe my patch improves [or at least my goal is to improve] compatability constrained by the fact that octave char is 8 bits [signedness system dependent], matlab appears to treat char as 16 bit [unsigned].

i assume changing octaves char type would be a major project. I would not propose such a change without support of the wider octave community.

Mike Sander <msander>
Wed 05 Feb 2014 08:52:07 PM UTC, comment #6:

Before making this another permanent Matlab-incompatibility, please consider, that that char is probably intended to stand for character, i.e. hold the numerical value for a character. In the long run, I would hope that Octave would move to Unicode encoding, and what value range would you give char then? I suggest using the same range as Matlab does.

Anonymous
Wed 05 Feb 2014 06:32:56 PM UTC, comment #5:

comment 4: good point regarding system dependence of char. with regards to the patch, should the tests be written as
std::numeric_limits<char>::min () ?

this way limits fall into the ranges [-128,127] or [0,255] as determined by signedness of char on that system. Is there a better approach?

In regards to matlab compatibility brought up in earlier comments, here is the matlab documentation for char():

So, octave has a char type which is sytem dependent, and 8 bits. matlab appears to treat char as signed & 16 bit. Richard Crozier (in linked discussion) confirmed negative values truncate to 0. he tested +1000 & matlab returned 1000 which is consistent with documentation.

In the current implementation of octave cast to char of negative is undefined. Cast to char of value > 127 results in 0. neither result is consistent with matlab.

Ultimately it comes down to requirements. If the requirement is identical behavior as matlab, then this implies reworking the internal char representation of octave. Seems unlikely to me.

If the requirement is to return a value limited to the range of char on the system in question, then I think the patch i provided is close to that. If the latter is the requirement, could i get some concrete feedback regarding the patch so I can make it right?

Mike Sander <msander>
Wed 05 Feb 2014 04:10:54 PM UTC, comment #4:

Internally, Octave uses the C type "char" to represent characters. Whether this is signed or unsigned is system dependent. On my system (GNU/Linux with GCC) it is signed. That it may sometimes appear to properly handle character codes greater than 127 is purely by chance.

John W. Eaton <jwe>
Project Administrator
Wed 05 Feb 2014 01:03:39 PM UTC, comment #3:

Part of what you say seems correct, but what I meant to
say is that checking with what Matlab does is important.
That is why I tried your examples in Matlab.

Michael Godfrey <godfrey>
Wed 05 Feb 2014 11:51:07 AM UTC, comment #2:

Michael, I did check. See http://octave.1599824.n4.nabble.com/cast-to-char-td4661457.html.

From matlab (online) documentation it treats char as unsigned 16 bit value. Octave (from what i can determine) treats char as unsigned 8 bit.

this patch removes the undefined result for negative inputs. It also returns 255 for values > 255, rather than 0. I believe these changes improve compatability, within the octave 8 bit char type.

if my arguments above are valid, are there any technical issues with the checks i have implemented?

Mike Sander <msander>
Wed 05 Feb 2014 03:27:15 AM UTC, comment #1:

In cases like this it is appropriate
to check for matlab behavior before making
proposals no matter how "reasonable" they may
seem.

Matlab (R2009B) says:

>> x=int8(-128)


x =

-128

>> z=char(x)

Warning: Out of range or non-integer values truncated during conversion to character.

z =

>> x=256


x =

256

>> z=char(x)


z =

>> single(z)


ans =

256

>>

====================
So, your suggestions will introduce further
Matlab incompatibility. It is not clear why
Matlab seems to think that 256 is within char range.

Michael Godfrey <godfrey>
Wed 05 Feb 2014 02:44:26 AM UTC, original submission:

I believe this should return 0. This assumes char maps to unsigned 8 bit type.

this should return 255.

The attached patch (cast_integer01.diff) has a proposed fix for integer types. I am open to comments/suggestions. Specifically is use of std::numeric_limits appropriate in this context?

NB. bug #38956 is related, but only considers +ve portion, not the undefined value.

Mike Sander <msander>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #30460:  cast_integer01.diff added by msander (1kB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by godfrey (Posted a comment)
  • -unavailable- added by msander (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 05 Feb 2014 02:44:26 AM UTCmsanderAttached File-=>Added cast_integer01.diff, #30460

    Back to the top


    Powered by Savane 3.1-cleanup