octave-dev — GNU Octave developers' list

Re: [OctDev] Failing tests in queueing

[Moreno M. <mo...@mo...>]

On 09/08/2012 05:39 PM, Claudio Freire wrote:
> A heads up: I don't use queueing. But I'm packaging it, meaning I
> build it and run its tests.
>
> I was updating from queueing-1.0.0 to queueing-1.1.0, and a lot of
> failing tests just appeared.
>
> I'm attaching a log.
>
> Most tests seem to fail due to numerical imprecision, though in some
> cases the error is quite big.

Thanks for your report. The cases where the error is big are known 
failures (in fact those blocks are marked with #!xtest, there are three 
of them). For the others I will increase the tolerance since they are 
due to numerical inaccuracy.
I will release a new version of queueing shortly.

Moreno.

-- 
Moreno Marzolla
EMail: mo...@mo...
WWW  : http://www.moreno.marzolla.name/

[OctDev] Failing tests in queueing

[Claudio F. <kla...@gm...>]

A heads up: I don't use queueing. But I'm packaging it, meaning I
build it and run its tests.

I was updating from queueing-1.0.0 to queueing-1.1.0, and a lot of
failing tests just appeared.

I'm attaching a log.

Most tests seem to fail due to numerical imprecision, though in some
cases the error is quite big.

Re: [OctDev] No isimag() curioso

[Michael D G. <mic...@gm...>]

On 09/07/2012 05:30 PM, Daniel J Sebald wrote:
> In working with the fftfilt() routine and wanting to know if the 
> inputs were purely imaginary I wondered why there is no such thing as 
> isimag().  It seems as though it would be as useful as or more useful 
> than iscomplex().
>
> Here's the thing: iscomplex() appears to be simply the complement of 
> isreal(), unless I'm missing a more sophisticated use of syntax:
>
> octave:5> x = 1
> x =  1
> octave:6> [isreal(x) iscomplex(x)]
> ans =
>
>    1   0
>
> octave:7> x = i
> x =  0 + 1i
> octave:8> [isreal(x) iscomplex(x)]
> ans =
>
>    0   1
>
> octave:9> x = 1+i
> x =  1 + 1i
> octave:10> [isreal(x) iscomplex(x)]
> ans =
>
>    0   1
>
> octave:11>
>
> I ask, What's the point of having a function that is simply !isreal()? 
> On the other hand isimag(), which is equivalent to "all (real (x) == 
> 0)) && !isreal (x)", would be a nice shorthand.
>
> Just an observation.  Usually duplication of function (or its 
> complement) is weeded out of programming languages.
>
> Dan 
I also recently noticed some quirks with complex:
x = 1 + 0i   is treated as real and not complex.
but
x = 0 + 1i  is treated as imag and complex.

But, this is consistent with Matlab.  However, Matlab has no iscomplex 
and also
differs in its example for help isreal.  There, Matlab gives an example:
        x = magic(3);
        y = complex(x);

which gives:
 >> y

y =

      8     1     6
      3     5     7
      4     9     2
 >> isreal(y)

ans =

      0
but, ~isreal(y) gives:
ans =

      1

The text in the help messages says that
isreal(y) returns
     false, because COMPLEX returns y with an all zero imaginary
     part.
-------------------------------------------
but Octave gives:

y =

    8 + 0i   1 + 0i   6 + 0i
    3 + 0i   5 + 0i   7 + 0i
    4 + 0i   9 + 0i   2 + 0i

and:
octave:21> isreal(y)
ans = 0
and:
octave:29> ~isreal(y)
ans =  1

The only difference is in the display of y.
This hardly qualifies as a bug, but does add a bit
to the confusion.  Some people would imagine
that real and imaginary would be treated "symmetrically."

So far I do not see much point in making a change.

[OctDev] Input/Output: io 1.0.20 released

[Carnë D. <car...@gm...>]

Hi everyone

a new release of io package is out, version 1.0.20, by Philip Nienhuis.
Enjoy Octave responsibly.

Carnë

[OctDev] No isimag() curioso

[Daniel J S. <dan...@ie...>]

In working with the fftfilt() routine and wanting to know if the inputs 
were purely imaginary I wondered why there is no such thing as isimag(). 
  It seems as though it would be as useful as or more useful than 
iscomplex().

Here's the thing: iscomplex() appears to be simply the complement of 
isreal(), unless I'm missing a more sophisticated use of syntax:

octave:5> x = 1
x =  1
octave:6> [isreal(x) iscomplex(x)]
ans =

    1   0

octave:7> x = i
x =  0 + 1i
octave:8> [isreal(x) iscomplex(x)]
ans =

    0   1

octave:9> x = 1+i
x =  1 + 1i
octave:10> [isreal(x) iscomplex(x)]
ans =

    0   1

octave:11>

I ask, What's the point of having a function that is simply !isreal()? 
On the other hand isimag(), which is equivalent to "all (real (x) == 0)) 
&& !isreal (x)", would be a nice shorthand.

Just an observation.  Usually duplication of function (or its 
complement) is weeded out of programming languages.

Dan

Re: [OctDev] Test failures due to tolerance in fftfilt.m

[Daniel J S. <dan...@ie...>]

On 09/06/2012 02:59 PM, Daniel J Sebald wrote:
> I'll toss this one to Ed and Rik, since we were just talking about
> precision issues for svds test failures...
>
> I checked the current state of tests and found this failure:
>
>>>>>> processing
>>>>>> /usr/local/src/octave/octave/octave/scripts/signal/fftfilt.m
> ***** test

There is a bit more to this, and I've put a patch on Savannah:

https://savannah.gnu.org/bugs/index.php?37297

The routine will round the output if the inputs are integers and will 
truncate the imaginary component if both inputs are real.  That seems 
fair, I suppose.  (I do wonder though if there should be an option to 
remove this behavior because some might not want such a thing.  Any 
thoughts maintainers or OctDev?)  I've extended that concept to account 
for the other cases of real*imaginary, imaginary*real, and 
imaginary*imaginary.  I don't see why only the real*real case should be 
done...all or nothing, as I see it.  These conditions now have tests, 
and there are a couple more tests for tolerance on the 
imaginary/imaginary scenario, as well as the complex/complex scenario.

By making the integerization (rounding) test more stringent, I uncovered 
a bug whereby only the first element of the output single row vector was 
rounded.

Dan

Re: [OctDev] openmpi_ext

[c. <car...@gm...>]

On 7 Sep 2012, at 14:18, Claudio Freire wrote:

> I thought of that, but that would be similar to
> 
> return octave_value_list();
> 
> Whereas in MPI_Probe.cc:105, theres another print_usage that returns
> octave_value(-1).
> 
> I thought it should return the same thing in both cases where it
> prints usage information.

returning an empty octave_value_list is the standard behaviour for 
other Octave functions, I left the -1 return value elswhere as it
was there already, but probably that should be changed too.

> 
> Anyway, I'm sure it builds with the patch you committed, I'll confirm
> a little bit later.
thanks!
c.

Re: [OctDev] bwlabeln "finished"

[Jordi G. H. <jo...@oc...>]

On 7 September 2012 07:06, Kris Thielemans <kri...@gm...> wrote:
> My initial impression if your newest bwlabeln implementation seems
> good!

Great, thanks for testing.

> I have only 1 problem. In my matlab code, I normally pass a (double)
> array with 0,1 to bwlabeln. This doesn't work for your
> implementation:
>
> bwlabeln(ones([2,2,3]))
> error: bwlabeln: first input argument must be a 'logical' ND-array

Okay, thanks, I've fixed this:

    http://octave.svn.sourceforge.net/viewvc/octave/trunk/octave-forge/main/image/src/bwlabeln.cc?r1=10978&r2=10977&pathrev=10978

- Jordi G. H.

Re: [OctDev] regionprops fixes n-dimensional fixes

[Jordi G. H. <jo...@oc...>]

On 6 September 2012 04:45, Søren Hauberg <so...@ha...> wrote:
>
> On Sep 5, 2012, at 11:51 PM, Jordi Gutiérrez Hermoso wrote:
>
>> regionprops does a bunch of things, most of which make sense for
>> N-dimensional images. I attach a proposed diff to address this. Can
>> anyone give me some review before I commit it?
>
>
> Your changes seem good. A few comments (though not a complete review):

I have just committed my changes:

    http://octave.svn.sourceforge.net/viewvc/octave/trunk/octave-forge/main/image/inst/regionprops.m?r1=10977&r2=10976&pathrev=10977

I ended up doing a lot more changes than the really bad diff I showed
you originally. I did some Nd indexing tricks there, and I used
nthargout, which is a function I wrote for Octave 3.6.x. I hope this
is acceptable.

> 1) You check if you should use 'bwlabel' or 'bwlabeln'. Shouldn't
> this check rather be part of 'bwlabeln', i.e. for 2D inputs,
> 'bwlabeln' can just call 'bwlabel' ?

In Matlab, bwlabel and bwlabeln use different algorithms. I didn't
analyse the bwlabel algorithm too closely, but I suppose it's better
suited for 2d images than the bwlabeln algorithm (union-find).

> 2) Not all properties are meaningful for ND input. The previous code
> checked that the input was 2D and gave an error otherwise. You have
> removed this check, which makes sense given that we now support ND
> input. But shouldn't there be some input checking for properties
> that only work for 2D input?

I think most properties make sense for ND input. I warned and refused
to return a result for those that seemed to make no sense for ND
input. These were only perimeter and orientation.

> 3) I guess the documentation needs to be updated as well?

Oh, yes. I suppose. I'll do that later. I gotta rush to make this work
for some deadline at work. ;-)

Since this seemed like a significant overhaul, I added myself to the
copyright line.

- Jordi G. H.

[OctDev] io-1.0.20 release

[Philip N. <pr....@hc...>]

Re: [OctDev] bwlabeln "finished"

[Kris T. <kri...@gm...>]

On 07 September 2012 12:55 Carnë Draug wrote
> On 7 September 2012 13:06, Kris Thielemans  wrote:
> > I have only 1 problem. In my matlab code, I normally pass a (double)
array
> with 0,1 to bwlabeln. This doesn't work for your implementation:
> >
> > bwlabeln(ones([2,2,3]))
> > error: bwlabeln: first input argument must be a 'logical' ND-array
> >
> > I can fix this by converting to bools of course
> >
> > bwlabeln(ones([2,2,3]~=0))
> >
> > but for matlab compatibility, ideally bwlabeln would do that itself. The
> same holds for bwlabel by the way.
> 
> What does matlab do if the double matrix has values different from 0 and
1?
> 

It seems to compare with 0 as I do above. For example, the following 2 tests
work in matlab 2011b

tmp=ones([2,3,4]);tmp(2,2,2)=.3;isequal(bwlabeln(tmp),ones(size(tmp)))
tmp=randn([2,3,4]);isequal(bwlabeln(tmp),tmp~=0)

Kris

Re: [OctDev] control 2.3.53 released

[Lukas R. <luk...@gm...>]

On 07.09.2012, at 13:57, marco atzeri wrote:

> On 9/7/2012 1:46 PM, Lukas Reichlin wrote:
>> On 07.09.2012, at 13:37, Rafael Laboissiere wrote:
>> 
>>> 
>>> I built the control package on my Debian system with libblas-dev
>>> installed and libatlas3-base, libatlas-dev, libopenblas-base, and
>>> libopenblas-dev removed.  I still get the same errors in the unit
>>> testings.  Here is the build log of the Debian package octave-control
>>> (version 2.3.53-1) with the same errors:
>>> 
>>> https://buildd.debian.org/status/fetch.php?pkg=octave-control&arch=i386&ver=2.3.53-1&stamp=1346708061
>>> 
>>> It seems that using the Reference BLAS is not enough for getting rid of
>>> the problem.
>>> 
>>> Rafael
>> 
>> You need Reference BLAS and Reference LAPACK. No ATLAS at all. Don't forget to rebuild Octave or to link control to the right libraries. If it still doesn't work, try to compile these libraries without optimizations.
>> 
>> Regards,
>> Lukas
> 
> on cygwin also with the Reference BLAS and Reference LAPACK the test
>  fails. As we are likely obtaining equivalent representations,
> what we can use as alternative test ?
> 
> Marco

Well, if only some signs change, use abs before comparing the matrices. If there are more general state transformations, I don't have a solution at hand. Maybe you can simply comment out the tests? (In case you don't like failing tests in packages maintained by you ;-)

Lukas

Re: [OctDev] openmpi_ext

[Claudio F. <kla...@gm...>]

On Fri, Sep 7, 2012 at 4:50 AM, c. <car...@gm...> wrote:
> ===================================================================
> --- MPI_Probe.cc        (revision 10967)
> +++ MPI_Probe.cc        (working copy)
> @@ -101,11 +101,8 @@
>            print_usage ();
>            results = octave_value (-1);
>          }
> -
> -      return results;
> -
> -      /* [ stat info ] = MPI_Probe (src, tag, comm) */
>      }
> +  return results;
>  }
>
>
> could you please check if this works for you?
> Thanks!

I thought of that, but that would be similar to

return octave_value_list();

Whereas in MPI_Probe.cc:105, theres another print_usage that returns
octave_value(-1).

I thought it should return the same thing in both cases where it
prints usage information.

Anyway, I'm sure it builds with the patch you committed, I'll confirm
a little bit later.

Re: [OctDev] openmpi_ext

[Claudio F. <kla...@gm...>]

On Fri, Sep 7, 2012 at 4:38 AM, c. <car...@gm...> wrote:
>> openmpi_ext fails to build with openmpi 1.5, since it uses an
>> undocumented, private field (_count) of the MPI_Status structure that
>> was renamed (to _ucount). It's still uncommitted, I had sent a patch
>> earlier. Just a reminder.
>
> It seems your patch went by unnoticed,
> where have you sent the patch? Is it still available somewhere?
> could you please send a link to it?
> Thanks,

Sure, it's attached to this reply.

Re: [OctDev] control 2.3.53 released

[marco a. <mar...@gm...>]

On 9/7/2012 1:46 PM, Lukas Reichlin wrote:
> On 07.09.2012, at 13:37, Rafael Laboissiere wrote:
>
>>
>> I built the control package on my Debian system with libblas-dev
>> installed and libatlas3-base, libatlas-dev, libopenblas-base, and
>> libopenblas-dev removed.  I still get the same errors in the unit
>> testings.  Here is the build log of the Debian package octave-control
>> (version 2.3.53-1) with the same errors:
>>
>> https://buildd.debian.org/status/fetch.php?pkg=octave-control&arch=i386&ver=2.3.53-1&stamp=1346708061
>>
>> It seems that using the Reference BLAS is not enough for getting rid of
>> the problem.
>>
>> Rafael
>
> You need Reference BLAS and Reference LAPACK. No ATLAS at all. Don't forget to rebuild Octave or to link control to the right libraries. If it still doesn't work, try to compile these libraries without optimizations.
>
> Regards,
> Lukas

on cygwin also with the Reference BLAS and Reference LAPACK the test
  fails. As we are likely obtaining equivalent representations,
what we can use as alternative test ?

Marco

Re: [OctDev] bwlabeln "finished"

[Carnë D. <car...@gm...>]

On 7 September 2012 13:06, Kris Thielemans <kri...@gm...> wrote:
> I have only 1 problem. In my matlab code, I normally pass a (double) array with 0,1 to bwlabeln. This doesn't work for your implementation:
>
> bwlabeln(ones([2,2,3]))
> error: bwlabeln: first input argument must be a 'logical' ND-array
>
> I can fix this by converting to bools of course
>
> bwlabeln(ones([2,2,3]~=0))
>
> but for matlab compatibility, ideally bwlabeln would do that itself. The same holds for bwlabel by the way.

What does matlab do if the double matrix has values different from 0 and 1?

Carnë

Re: [OctDev] control 2.3.53 released

[Lukas R. <luk...@gm...>]

On 07.09.2012, at 13:37, Rafael Laboissiere wrote:

> * Lukas Reichlin <luk...@gm...> [2012-09-03 12:03]:
> 
>> On 03.09.2012, at 10:39, Rafael Laboissiere <ra...@la...> wrote:
>> 
>>> * Rafael Laboissiere <ra...@la...> [2012-08-30 23:47]:
>>> 
>>>> * Carnë Draug <car...@gm...> [2012-08-27 16:43]:
>>>> 
>>>>> a new release of control package is out, version 2.3.53, by Lukas Reichlin.
>>>> 
>>>> In exercising the tests in inst/@lti/minreal.m, I got the error below.
>>>> Is it normal?
>>>> 
>>>> I am running octave 3.6.2 on a Debian unstable system.
>>>> 
>>>> [snip]
>>> 
>>> Testing ltimodels and bstmodred also yield errors, cf below.
>>> 
>>> [snip]
>> 
>> You can check whether the observed and expected results are equivalent
>> state-space models (i.e. state-transformation, see command prescale for
>> formulae). This can be done, e.g., by inspection of the Hankel singular
>> values (command hsvd), time response (step, impulse) or frequency
>> response (sigma).
>> 
>> If they are the same, there should be nothing to worry about. If you
>> want the same results, use Reference BLAS (and LAPACK) from
>> www.netlib.org instead of ATLAS which you are probably using. The
>> SLICOT authors recommend the use of the reference implementations.
>> Correct results are more important than minor speed advantages of
>> automatically tuned linear algebra software, aren't they? :-)
> 
> I built the control package on my Debian system with libblas-dev
> installed and libatlas3-base, libatlas-dev, libopenblas-base, and
> libopenblas-dev removed.  I still get the same errors in the unit
> testings.  Here is the build log of the Debian package octave-control
> (version 2.3.53-1) with the same errors:
> 
> https://buildd.debian.org/status/fetch.php?pkg=octave-control&arch=i386&ver=2.3.53-1&stamp=1346708061
> 
> It seems that using the Reference BLAS is not enough for getting rid of
> the problem.
> 
> Rafael

You need Reference BLAS and Reference LAPACK. No ATLAS at all. Don't forget to rebuild Octave or to link control to the right libraries. If it still doesn't work, try to compile these libraries without optimizations.

Regards,
Lukas

Re: [OctDev] control 2.3.53 released

[Rafael L. <ra...@la...>]

* Lukas Reichlin <luk...@gm...> [2012-09-03 12:03]:

> On 03.09.2012, at 10:39, Rafael Laboissiere <ra...@la...> wrote:
> 
> > * Rafael Laboissiere <ra...@la...> [2012-08-30 23:47]:
> > 
> >> * Carnë Draug <car...@gm...> [2012-08-27 16:43]:
> >> 
> >>> a new release of control package is out, version 2.3.53, by Lukas Reichlin.
> >> 
> >> In exercising the tests in inst/@lti/minreal.m, I got the error below.
> >> Is it normal?
> >> 
> >> I am running octave 3.6.2 on a Debian unstable system.
> >> 
> >> [snip]
> > 
> > Testing ltimodels and bstmodred also yield errors, cf below.
> > 
> > [snip]
> 
> You can check whether the observed and expected results are equivalent
> state-space models (i.e. state-transformation, see command prescale for
> formulae). This can be done, e.g., by inspection of the Hankel singular
> values (command hsvd), time response (step, impulse) or frequency
> response (sigma).
>
> If they are the same, there should be nothing to worry about. If you
> want the same results, use Reference BLAS (and LAPACK) from
> www.netlib.org instead of ATLAS which you are probably using. The
> SLICOT authors recommend the use of the reference implementations.
> Correct results are more important than minor speed advantages of
> automatically tuned linear algebra software, aren't they? :-)

I built the control package on my Debian system with libblas-dev
installed and libatlas3-base, libatlas-dev, libopenblas-base, and
libopenblas-dev removed.  I still get the same errors in the unit
testings.  Here is the build log of the Debian package octave-control
(version 2.3.53-1) with the same errors:

https://buildd.debian.org/status/fetch.php?pkg=octave-control&arch=i386&ver=2.3.53-1&stamp=1346708061

It seems that using the Reference BLAS is not enough for getting rid of
the problem.

Rafael

Re: [OctDev] bwlabeln "finished"

[Kris T. <kri...@gm...>]

Hi Jordi

My initial impression if your newest bwlabeln implementation seems good! It's not so easy to test this exhaustively of course. But for instance this works

tmp=ones(2,2,3);
% add another "blob" somewhere further away
tmp(3:4, 4:6, 2:3)=ones(2,3,2);
res=bwlabeln(tmp~=0);
% we should have found 2 blobs
expectedres=ones(2,2,3);
expectedres(3:4, 4:6, 2:3)=ones(2,3,2)*2;
assert(res, expectedres)

The test above is somewhat problematic, as bwlabeln could just as well have found the blobs in different order. I haven't bothered to come up with a better test code.

I have only 1 problem. In my matlab code, I normally pass a (double) array with 0,1 to bwlabeln. This doesn't work for your implementation:

bwlabeln(ones([2,2,3]))
error: bwlabeln: first input argument must be a 'logical' ND-array

I can fix this by converting to bools of course

bwlabeln(ones([2,2,3]~=0))

but for matlab compatibility, ideally bwlabeln would do that itself. The same holds for bwlabel by the way.

Thanks for this!

Kris
> -----Original Message-----
> From: jo...@gm... [mailto:jo...@gm...] On Behalf Of Jordi
> Gutiérrez Hermoso
> Sent: 05 September 2012 21:15
> To: Kris Thielemans
> Cc: Octave Forge List
> Subject: Re: [OctDev] bwlabeln "finished"
> 
> Just a heads up to everyone, I've improved the bwlabeln code a bit to
> make it faster. Please let me know how it works for you.
> 
> TIA,
> - Jordi G. H.

[OctDev] fpl-1.3.2

[Carlo de F. <car...@gm...>]

Hi,

I just uploaded version 1.3.2 of the fpl package to the release tracker.
Before the release is made official, I'd be grateful if someone with an
older
version of Octave installed could confirm thatwith the latest changes
compatibility
with Octave 3.2.* has been recovered.

To test, please install (and load) this package and msh and then run the
following commands:

x = linspace (0, 1, 10);
msh = msh3m_structured_mesh (x, x, x, 1, 1:6);
u = msh.p(1,:)';
fpl_vtk_raw_write_field ("pippo1", msh, {u,"u"}, {})
fpl_vtk_b64_write_field ("pippo2", msh, {u,"u"}, {})


Thanks!
c.

Re: [OctDev] openmpi_ext

[c. <car...@gm...>]

On 7 Sep 2012, at 06:58, Claudio Freire wrote:

> Regarding this patch, it seems there's a missing return
> that makes
> newer gcc complain about garbage being returned - there's a patch for
> that attached.

Thanks for reporting this.
It seems the bug was not a missing return but rather a misplaced parentesys so I checked in this patch instead:

===================================================================
--- MPI_Probe.cc	(revision 10967)
+++ MPI_Probe.cc	(working copy)
@@ -101,11 +101,8 @@
           print_usage ();
           results = octave_value (-1);
         }
-
-      return results;
-      
-      /* [ stat info ] = MPI_Probe (src, tag, comm) */
     }
+  return results;
 }


could you please check if this works for you?
Thanks!
c.

Re: [OctDev] openmpi_ext

[c. <car...@gm...>]

On 7 Sep 2012, at 06:58, Claudio Freire wrote:

> openmpi_ext fails to build with openmpi 1.5, since it uses an
> undocumented, private field (_count) of the MPI_Status structure that
> was renamed (to _ucount). It's still uncommitted, I had sent a patch
> earlier. Just a reminder.

It seems your patch went by unnoticed,
where have you sent the patch? Is it still available somewhere?
could you please send a link to it?
Thanks,
c.

Re: [OctDev] openmpi_ext

[Claudio F. <kla...@gm...>]

On Tue, Aug 21, 2012 at 11:48 PM, c. <car...@gm...> wrote:
>>> I went through the code and did a little cleanup, fixed some trivial bugs and removed
>>> a lot of code duplication making use of templates and macros where possible.
>>>
>>> Does anyone object if I apply the attached patch and prepare a new release?
>>
>> Seems no one has anything against it, so I'd say "Go for it".
>>
>> Carnë
>
> Just committed changes to the repository I'll make the release over the weekend,
> c.

openmpi_ext fails to build with openmpi 1.5, since it uses an
undocumented, private field (_count) of the MPI_Status structure that
was renamed (to _ucount). It's still uncommitted, I had sent a patch
earlier. Just a reminder.

Regarding this patch, it seems there's a missing return that makes
newer gcc complain about garbage being returned - there's a patch for
that attached.

Have fun.

Re: [OctDev] regionprops fixes n-dimensional fixes

[Søren H. <so...@ha...>]

On Sep 5, 2012, at 11:51 PM, Jordi Gutiérrez Hermoso wrote:

> regionprops does a bunch of things, most of which make sense for
> N-dimensional images. I attach a proposed diff to address this. Can
> anyone give me some review before I commit it?


Your changes seem good. A few comments (though not a complete review):

1) You check if you should use 'bwlabel' or 'bwlabeln'. Shouldn't this check rather be part of 'bwlabeln', i.e. for 2D inputs, 'bwlabeln' can just call 'bwlabel' ?

2) Not all properties are meaningful for ND input. The previous code checked that the input was 2D and gave an error otherwise. You have removed this check, which makes sense given that we now support ND input. But shouldn't there be some input checking for properties that only work for 2D input?

3) I guess the documentation needs to be updated as well?

Other then that, I think your patch looks great!

Søren

Re: [OctDev] Ode23s

[c. <kin...@ti...>]

On 6 Sep 2012, at 09:13, c. wrote:

> 
> On 5 Sep 2012, at 23:31, davide prandi wrote:
> 
>> Well, it works. Thanks to everyone! I'll load it on sourceforge as soon as possible, if someone wants to check/test it I'll be happy.
>> Regards,
>> 
>> Davide
> 
> Davide,
> Some suggestions for your next version:
> 
> - the copyright notice is incomplete. You can look at other 
>   files in Octave Forge for examples, you can browse other packages' sorce code from here:
>   octave.svn.sourceforge.net/viewvc/octave/trunk/octave-forge/
> 
>   In particular it should be at the top of the file, before the function signature and it 
>   should start with "%% Copyright (C)", Octave looks for this string to understand that it
>   is a copyright notice and not part of the help text.
> 
> - If you have no special reason for doing otherwise I'd reccomend using GPL "version 3 or later" as the 
>  license.
> 
> - In order to make testing easier you may want to add some automatic tests to your file, 
>  these are code blocks (usually placed at the end of the file) commented with the special symbol "%!"
>  and starting with "%!test". If your function contains such automatic tests, the command  "test ode23s"
>  will run all the tests and report the results as PASS or FAIL.
>  See here for more details: 
>  http://www.gnu.org/software/octave/doc/interpreter/Test-Functions.html#Test-Functions
>  A good test for your function would be to check the order of convergence and stability of the method
>  on a linear problem for which an exact solution is known.
> 
> -  If you write examples/test that contain graphical output, it's better to include them as demos rather than tests.
>   In your case it would make sense to plot the exact and numerical results for comparison.
> 
> - You should start adding a documentation string, see here for tips about how to write such string:
>  http://www.gnu.org/software/octave/doc/interpreter/Documentation-Tips.html#Documentation-Tips
> 
> - When you upload the next version to the tracker also send a message to this mailing list asking for comments
>  as almost nobody monitors the tracker. You might also want to cc the maintainer of "odepkg", Thomas Treichl, 
>  as I think that package is the best place where to place your function.
> 
> Welcome to Octave and keep up the good work,
> Carlo


Another comment:
 
the correct way to check whether a variable is empty is not

if size(options.RelTol)~=size([])

instead use

if (! isempty (options.RelTol))

to make the check more robust you could also check whether the field exists before checking for its size

if (if isfield ("options", "RelTol") && ! isempty (options.RelTol))

HTH,
c.