Re: std in NDArrays

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

David Bateman-3
I'm moving this discussion to the maintainers list..

According to John W. Eaton <[hidden email]> (on 08/26/04):
>   * bitcmp, bitget, and bitset should work for int64 types (currently
>     they won't because they rely on being able to convert to double
>     and/or perform arithmetic on the values.  It would be best to
>     avoid using eval in these functions.  Perhaps they should be
>     implemented in C++?

Addressing this point, the only reason the existing bitcmp, bitget and
bitset functions don't work for uint64 and int64 is lines like

    Amax = eval ([cname, " (log2 (double (intmax (cname))) + 1);"]);

in the code. I originally thought that it would make sense to convert the
log mapper functions for use with the integer types. However it turns out
that this is quite complex and as they would only return the integer part
of the log and log2(x) is implemented as log(x)/log(2) and the integer
part of log(2) is zero, this doesn't make any sense...

The only other mapper functions that might be of use are "abs" and "sign",
so I really don't think it is worth modifying the octave_mapper class for
this...

However as the value Amax is just the number of bits in the integer
representation the above line might be written as

    Amax = str2num(substr(cname, 5, length(cname)-4));

and this will then work for uint64. A similar change is needed for the
int64 case. With only this change the existing functions will work with
int64 and uint64 with a small proviso as discussed below.

As for the other "eval" statements, they are all a variant of the form
"eval ([cname, "(1);"]);" and are used to cast an unknown type to the
correct type. I think these are pretty harmless, for readability I can
write

_conv = eval (["@", cname, ";"]);

and hide the other evals behind this function handle. This is certainly
clearer..

Note that for n > 53, bitset and bitget will be broken due to the
incorrect return type of .^, and so this needs to be fixed..

Also note that something like

a = uint8 (int8 (1))

or even

a = uint8 (uint64 (1))

is currently broken as they return an int8 type rather than a uint8 type...
I fixed this bug in passing...

Find attached a patch that addresses all of the above issues and thus
gets bitcmp, bitget and bitset working for both int64 and uint64 types.
Hopefully, this brings a 2.1.58 release that little bit closer..

D.

PS. John, I've seen no reaction from you to my patches in the mails

http://www.octave.org/mailing-lists/bug-octave/2004/582
http://www.octave.org/mailing-lists/octave-maintainers/2004/627

I know these are relatively recent patches, but as you're talking about
a 2.1.58 release just giving you a heads up for some of my other patches
to perhaps include...
 
--
David Bateman                                [hidden email]
Motorola CRM                                 +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 1 69 35 77 01 (Fax)
91193 Gif-Sur-Yvette FRANCE

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

patch.bitfcns20040826 (19K) Download Attachment
changelog-bitfcns20040826 (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

John W. Eaton-6
On 26-Aug-2004, David Bateman <[hidden email]> wrote:

| I'm moving this discussion to the maintainers list..

OK.

| According to John W. Eaton <[hidden email]> (on 08/26/04):
| >   * bitcmp, bitget, and bitset should work for int64 types (currently
| >     they won't because they rely on being able to convert to double
| >     and/or perform arithmetic on the values.  It would be best to
| >     avoid using eval in these functions.  Perhaps they should be
| >     implemented in C++?
|
| Addressing this point, the only reason the existing bitcmp, bitget and
| bitset functions don't work for uint64 and int64 is lines like

I was really thinking about rewriting these operations in C++ in a way
that would avoid the need to do any evals or funny operations on the
names of the types.

Also, I don't think we should do any arithmetic on octave_int64 types
as long as the rules for those operations are not the same as for the
other octave_intX types because there is no simple way to implement the
saturation rules.

jwe


Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

John W. Eaton-6
In reply to this post by David Bateman-3
On 26-Aug-2004, David Bateman <[hidden email]> wrote:

| Thanks for thr ToDo list. Anyone else want to attack some of these?

I intend to work on the fwrite/fread problems first.

jwe


Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

David Bateman-3
In reply to this post by John W. Eaton-6
According to John W. Eaton <[hidden email]> (on 08/26/04):
> I was really thinking about rewriting these operations in C++ in a way
> that would avoid the need to do any evals or funny operations on the
> names of the types.

The problem with rewritting them in C++ is that you'd have to effectively
reimplement the the bitand, bitor and bitxor functionality within these
seperate functions. As script files we can use the existing bitand, etc
functions more easily. This also reduces the risk of getting things wrong.

There is now only a single "eval" in these functions, that is really quite
clear...

Ok, funny operations on type names I'll give you.. But this is no reason to
convert to C++. If you really really want to get rid of both eval and the
funny ops on typenames, then something like

  if strcmp (cname, "double")
    Bmax = bitmax;
    Amax = log2 (Bmax) + 1;
    _conv = @double;
  else
    if strcmp (cname, "uint8")
      Amax = 8;
      _conv = @uint8;
    elseif strcmp (cname, "uint16")
      Amax = 16;
      _conv = @uint16;
    elseif strcmp (cname, "uint32")
      Amax = 32;
      _conv = @uint32;
    elseif strcmp (cname, "uint64")
      Amax = 64;
      _conv = @uint64;
    elseif strcmp (cname, "int8")
      Amax = 8;
      _conv = @int8;
    elseif strcmp (cname, "int16")
      Amax = 16;
      _conv = @int16;
    elseif strcmp (cname, "int32")
      Amax = 32;
      _conv = @int32;
    elseif strcmp (cname, "int64")
      Amax = 64;
      _conv = @int64;
    else
      error ("bitcmp: invalid class %s", cname);
    endif
    Bmax = intmax (cname);
  endif

will get rid of all of it, but at the cost of longer and to me a less
clear code block...

>
> Also, I don't think we should do any arithmetic on octave_int64 types
> as long as the rules for those operations are not the same as for the
> other octave_intX types because there is no simple way to implement the
> saturation rules.

As for saturations, the only arithmetic operations on the integer types
are of the form

   Bmax - _conv (_conv (2) .^ (_conv (n) - Aone))

The test

  m = _conv (n(:));
  if (any (m < Aone) || any (m > Amax))
    error ("n must be in the range [1,%d]", Amax);
  endif

causes an error for the values of n that would cause any problems. So there
can be no saturate as this is implicitly an error.

Regards
David

--
David Bateman                                [hidden email]
Motorola CRM                                 +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 1 69 35 77 01 (Fax)
91193 Gif-Sur-Yvette FRANCE

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary


Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

David Bateman-3
Probably better to use "isa" rather than strcmp... Ok, I've rewritten
my previous patch taking into account your comment on the eval and funny
operations. Find it attached, though I think the other solution is neater...

Cheers
David

--
David Bateman                                [hidden email]
Motorola CRM                                 +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 1 69 35 77 01 (Fax)
91193 Gif-Sur-Yvette FRANCE

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

patch.bitfcns20040826-1 (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

David Bateman-3
In reply to this post by John W. Eaton-6
According to John W. Eaton <[hidden email]> (on 08/26/04):
> On 26-Aug-2004, David Bateman <[hidden email]> wrote:
>
> | Thanks for thr ToDo list. Anyone else want to attack some of these?
>
> I intend to work on the fwrite/fread problems first.

John,

As I believe the bitcmp, bitget, bitset issues are completely
addressed by my last patch (patch.bitfcns20040826-3), I attacked the
problem of "format hex", etc. I also at the same time fixed up the
column alignment of the integer types..

Please find the patch and change log attached... Since you are
attacking the fread issues and together with my patch for
bit{cmp|get|set}, the only issue remaining on your todo list for a
2.1.58 release is the mixed type double scalar and integer
operations. Not sure what you want here though..

D.

--
David Bateman                                [hidden email]
Motorola CRM                                 +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 1 69 35 77 01 (Fax)
91193 Gif-Sur-Yvette FRANCE

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

changelog-pr-output (568 bytes) Download Attachment
patch.pr-output (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: std in NDArrays

John W. Eaton-6
On 27-Aug-2004, David Bateman <[hidden email]> wrote:

| As I believe the bitcmp, bitget, bitset issues are completely
| addressed by my last patch (patch.bitfcns20040826-3),

I will look at that one next.

| I attacked the
| problem of "format hex", etc. I also at the same time fixed up the
| column alignment of the integer types..
|
| Please find the patch and change log attached...

OK, I applied the most recent version of this patch that you sent
after our discussions.

| Since you are
| attacking the fread issues and together with my patch for
| bit{cmp|get|set}, the only issue remaining on your todo list for a
| 2.1.58 release is the mixed type double scalar and integer
| operations. Not sure what you want here though..

For compatibility, we need to make it possible to write things like

  x = uint8 (16);
  y = 2 * x;

and have the result be a uint8 object.  Matlab only does this for
double scalars, presumably because all literal scalar constants in the
language are converted to double values on input, but it is somewhat
inconvenient to be forced to write

  y = uint8 (2) * x;

and also a bit of a problem since mixed-type integer ops are not
supported, so you then have to know the type of the rest of the
expression to be able to multiply by a literal scalar...

I will take a shot at implementing this next.

Thanks,

jwe