Deprecate sizemax() ?

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

Deprecate sizemax() ?

Rik-4
All,

I was investigating a very old bug where the ones() function was able (sort
of) to create an array where the number of elements was greater than
sizemax() (see https://savannah.gnu.org/bugs/index.php?47468).  It turns
out that Octave was doing something rather clever.  For row vectors (1xN
objects), ones() was substituting a Range object rather than an NDArray.  A
Range object requires constant storage (24B) versus an Array object (8 *
numel (x)).  Since constant arrays are often built by C*ones(...), this has
more applicability than one might think.  This seemed like a good feature
that should apply to all of the constant array functions such as zeros(),
pi(), e() as well so I made that change
(https://hg.savannah.gnu.org/hgweb/octave/rev/e78b6e7f743c).

However, now there is the remaining problem that all of these function can
create arrays larger than sizemax().  Is the sizemax() constraint, as
opposed to the limit of the underlying representation which is
octave_idx_type and is either intmax ('int32') or intmax ('int64')
depending on how Octave was built, truly necessary anymore?  There is
another bug report that says the value of sizemax is off by one
(https://savannah.gnu.org/bugs/?47469) which is related.  Originally, there
seems to have been a desire to facilitate the construction of 2-D sparse
arrays from full arrays.  But my thinking is that the implementation of one
particular object type should not affect the API for all Octave object's. 
If the hard requirement for sparse arrays is that numel (x) <= intmax
(octave_idx_type) - 1 then that can be checked in the constructor for those
objects, but other routines shouldn't require this.

This is foundational--a change to the API--which is why I'm asking about it
on the Maintainer's list.  In practice, this isn't much of an issue.  Most
modern builds enable 64-bit indexing and the difference between being able
to create arrays with 2^63 - 1 elements versus 2^63 - 2 is largely
theoretical.  After all, there are few machines that have 2^63 * 8B =
7.38e19 B of RAM available.

--Rik



Reply | Threaded
Open this post in threaded view
|

Re: Deprecate sizemax() ?

siko1056
On 10/28/19 2:46 AM, Rik wrote:

> All,
>
> I was investigating a very old bug where the ones() function was able (sort
> of) to create an array where the number of elements was greater than
> sizemax() (see https://savannah.gnu.org/bugs/index.php?47468).  It turns
> out that Octave was doing something rather clever.  For row vectors (1xN
> objects), ones() was substituting a Range object rather than an NDArray.  A
> Range object requires constant storage (24B) versus an Array object (8 *
> numel (x)).  Since constant arrays are often built by C*ones(...), this has
> more applicability than one might think.  This seemed like a good feature
> that should apply to all of the constant array functions such as zeros(),
> pi(), e() as well so I made that change
> (https://hg.savannah.gnu.org/hgweb/octave/rev/e78b6e7f743c).
>
> However, now there is the remaining problem that all of these function can
> create arrays larger than sizemax().  Is the sizemax() constraint, as
> opposed to the limit of the underlying representation which is
> octave_idx_type and is either intmax ('int32') or intmax ('int64')
> depending on how Octave was built, truly necessary anymore?  There is
> another bug report that says the value of sizemax is off by one
> (https://savannah.gnu.org/bugs/?47469) which is related.  Originally, there
> seems to have been a desire to facilitate the construction of 2-D sparse
> arrays from full arrays.  But my thinking is that the implementation of one
> particular object type should not affect the API for all Octave object's. 
> If the hard requirement for sparse arrays is that numel (x) <= intmax
> (octave_idx_type) - 1 then that can be checked in the constructor for those
> objects, but other routines shouldn't require this.
>
> This is foundational--a change to the API--which is why I'm asking about it
> on the Maintainer's list.  In practice, this isn't much of an issue.  Most
> modern builds enable 64-bit indexing and the difference between being able
> to create arrays with 2^63 - 1 elements versus 2^63 - 2 is largely
> theoretical.  After all, there are few machines that have 2^63 * 8B =
> 7.38e19 B of RAM available.
>
> --Rik
>

Sorry for coming late to this.  To give one opinion, I do not mind
removing the sizemax() constraint, which only a few/one? function(s)
honestly regard(s), for the reasons you mentioned.  All functions
creating objects of any type should take care, that their created
product is usable and stays within the natural borders given mainly by
the octave_idx_type and, if possible, the available main memory.  Your
change https://hg.savannah.gnu.org/hgweb/octave/rev/e78b6e7f743c
perfectly exploits those borders and why should ones() bother for a
potential conversion to a sparse matrix?  If such a conversion happens,
then the respective target type construction method should be
responsible for possible limitations of that conversion.

Switching to 64 bit indexing was a real game changer, thus you are
right, the first limitations we will hit in the next years is our main
memory.

My suggestion for Octave 6 as mentioned in
https://savannah.gnu.org/bugs/?47469:

a) deprecate libinterp/corefcn/bitfcns.cc (Fsizemax) and
   remove it's documentation entry
b) deprecate dim_vector::dim_max (), which is exclusively used for a)
c) close report #47469 as won't fix
d) close report #47468 as fixed by you Rik

Best,
Kai

Reply | Threaded
Open this post in threaded view
|

Re: Deprecate sizemax() ?

Rik-4
In reply to this post by Rik-4
On 10/30/2019 11:54 PM, [hidden email] wrote:
Subject:
Re: Deprecate sizemax() ?
From:
Kai Torben Ohlhus [hidden email]
Date:
10/30/2019 10:07 PM
To:
[hidden email]
List-Post:
[hidden email]
Content-Transfer-Encoding:
8bit
Precedence:
list
MIME-Version:
1.0
References:
<MTAwMDAxMC5ub21hZA.1572198391@quikprotect>
In-Reply-To:
<MTAwMDAxMC5ub21hZA.1572198391@quikprotect>
Message-ID:
[hidden email]
Content-Type:
text/plain; charset=utf-8
Message:
2

On 10/28/19 2:46 AM, Rik wrote:
All,

I was investigating a very old bug where the ones() function was able (sort
of) to create an array where the number of elements was greater than
sizemax() (see https://savannah.gnu.org/bugs/index.php?47468).  It turns
out that Octave was doing something rather clever.  For row vectors (1xN
objects), ones() was substituting a Range object rather than an NDArray.  A
Range object requires constant storage (24B) versus an Array object (8 *
numel (x)).  Since constant arrays are often built by C*ones(...), this has
more applicability than one might think.  This seemed like a good feature
that should apply to all of the constant array functions such as zeros(),
pi(), e() as well so I made that change
(https://hg.savannah.gnu.org/hgweb/octave/rev/e78b6e7f743c).

However, now there is the remaining problem that all of these function can
create arrays larger than sizemax().  Is the sizemax() constraint, as
opposed to the limit of the underlying representation which is
octave_idx_type and is either intmax ('int32') or intmax ('int64')
depending on how Octave was built, truly necessary anymore?  There is
another bug report that says the value of sizemax is off by one
(https://savannah.gnu.org/bugs/?47469) which is related.  Originally, there
seems to have been a desire to facilitate the construction of 2-D sparse
arrays from full arrays.  But my thinking is that the implementation of one
particular object type should not affect the API for all Octave object's. 
If the hard requirement for sparse arrays is that numel (x) <= intmax
(octave_idx_type) - 1 then that can be checked in the constructor for those
objects, but other routines shouldn't require this.

This is foundational--a change to the API--which is why I'm asking about it
on the Maintainer's list.  In practice, this isn't much of an issue.  Most
modern builds enable 64-bit indexing and the difference between being able
to create arrays with 2^63 - 1 elements versus 2^63 - 2 is largely
theoretical.  After all, there are few machines that have 2^63 * 8B =
7.38e19 B of RAM available.

--Rik

Sorry for coming late to this.  To give one opinion, I do not mind
removing the sizemax() constraint, which only a few/one? function(s)
honestly regard(s), for the reasons you mentioned.  All functions
creating objects of any type should take care, that their created
product is usable and stays within the natural borders given mainly by
the octave_idx_type and, if possible, the available main memory.  Your
change https://hg.savannah.gnu.org/hgweb/octave/rev/e78b6e7f743c
perfectly exploits those borders and why should ones() bother for a
potential conversion to a sparse matrix?  If such a conversion happens,
then the respective target type construction method should be
responsible for possible limitations of that conversion.

Switching to 64 bit indexing was a real game changer, thus you are
right, the first limitations we will hit in the next years is our main
memory.

My suggestion for Octave 6 as mentioned in
https://savannah.gnu.org/bugs/?47469:

a) deprecate libinterp/corefcn/bitfcns.cc (Fsizemax) and
   remove it's documentation entry
b) deprecate dim_vector::dim_max (), which is exclusively used for a)
c) close report #47469 as won't fix
d) close report #47468 as fixed by you Rik
That's pretty much what I'd like to do.  If there are no opinions against this I will go ahead and do it.

--Rik

Reply | Threaded
Open this post in threaded view
|

Re: Deprecate sizemax() ?

John W. Eaton
Administrator
On 10/31/19 12:14 PM, Rik wrote:
> Kai Torben Ohlhus <[hidden email]> wrote:

>> My suggestion for Octave 6 as mentioned in
>> https://savannah.gnu.org/bugs/?47469:
>>
>> a) deprecate libinterp/corefcn/bitfcns.cc (Fsizemax) and
>>     remove it's documentation entry
>> b) deprecate dim_vector::dim_max (), which is exclusively used for a)
>> c) close report #47469 as won't fix
>> d) close report #47468 as fixed by you Rik

> That's pretty much what I'd like to do.  If there are no opinions
> against this I will go ahead and do it.

No objection from me.

Thanks,

jwe