issues/remarks on the trunk code

Dear all,

I took an extensive look the BFL code (trunk).

I have some issues/remarks which I would like to share.

1) MCPF - strange construction
==============================

Strange construction in MCPdf. Assignment of numsamples can more efficient.
Currently:

template <typename T> bool
MCPdf<T>::ListOfSamplesUpdate(const vector<Sample<T> > & los)
{
unsigned int numsamples = _listOfSamples.size();
if ((numsamples = los.size()) == _listOfSamples.size())

2) Test case sensitive to result of random number generator
===========================================================
Several test-cases will fail, because they are sensitive to the result of
the Random number generator.
Is it not possible to use a fixed seed for these test-cases. Then this
testcases are less sensitive. In the future
a continuous integration process can be introduced.

3) 32 bit versus 64 bit
=======================
I saw that some test-case seems to rely on the fact that the used type for
matrix calculations is a double.
I worked on a 32 bit system. I wondering if I use a 64 bit double, will the
system (test-cases) still be working?

4) Hiding member of parent
==========================
The member _old_samples in EKParticleFilter hides the member _old_samples of
ParticleFilter

5) Example/TestKallmanSmoother
==============================
The example returns (line 310) before closing the files.

With best regards,

Jürge
_______________________________________________
I hereby promise not to top-post on the
BFL mailing list
BFL [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/bfl

issues/remarks on the trunk code

On Tue, Sep 1, 2009 at 8:07 AM, Jurge van Eijck
<jurge [dot] van [dot] eijck [..] ...> wrote:
> I took an extensive look the BFL code (trunk).
>
> I have some issues/remarks which I would like to share.
>
> 1) MCPF - strange construction
> ==============================
>
> Strange construction in MCPdf. Assignment of numsamples can more efficient.
> Currently:
>
>
>  template <typename T> bool
>    MCPdf<T>::ListOfSamplesUpdate(const vector<Sample<T> > & los)
>    {
>      unsigned int numsamples = _listOfSamples.size();
>      if ((numsamples = los.size()) == _listOfSamples.size())

should be fixed now

> 2) Test case sensitive to result of random number generator
> ===========================================================
> Several test-cases will fail, because they are sensitive to the result of
> the Random number generator.
> Is it not possible to use a fixed seed for these test-cases. Then this
> testcases are less sensitive. In the future
> a continuous integration process can be introduced.

We are aware of the problem (bug 491 is not completely unrelated).
I'm not sure using a fixed-seed is the best solution (you might hide
other problems than) though. If you have any thoughts on how we can
improve the test coverage, they are very welcome though.
It is probably a good idea to put this in bugzilla so we don't forget it!

> 3) 32 bit versus 64 bit
> =======================
> I saw that some test-case seems to rely on the fact that the used type for
> matrix calculations is a double.
> I worked on a 32 bit system. I wondering if I use a 64 bit double, will the
> system (test-cases) still be working?

I think tinne (or ruben) might be running BFL on a 64 bit system and I
didn't hear them complaining :-)

> 4) Hiding member of parent
> ==========================
> The member _old_samples in EKParticleFilter hides the member _old_samples of
> ParticleFilter

Should be fixed.

> 5) Example/TestKallmanSmoother
> ==============================
> The example returns (line 310) before closing the files.

Should be fixed.

Thx for your remarks, and sorry for the slow throughput!

Klaas
_______________________________________________
I hereby promise not to top-post on the
BFL mailing list
BFL [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/bfl