For more infomation about this bug, visit

--- Comment #3 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-07 10:14:17 ---
Some additional thoughts,

If the dimensionSet is used as a public function, it seems to me it's only
usefull in the case the pdf under consideration is not initialized yet.
Therefor, it could also be usefull to add a private boolean _initialized,
representing if the pdf is initialized or not.
The dimensionSet could then be changed such that it is only possible to call
this function provided _initialized == false.

Remarks?

Tinne

For more infomation about this bug, visit

Tinne De Laet <tinne [dot] delaet [..] ...> changed:

What |Removed |Added
--------------------------------------------------------------------------
CC| |tinne [dot] delaet [..] ...en.b
| |e

--- Comment #2 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-07 09:50:57 ---
>> This was written simultaneously with Klaas' comment.

First I want to open a discussion on the dimensionSet function itself.
As implemented now, the dimensionSet function is a public function (with
template code in pdf.h).
The only time so far that dimensionSet is used as a public function is in
filterproposaldensity.cpp:
_TmpPrior->DimensionSet(SysModel->StateSizeGet());

I believe it is safer to make dimensionSet protected, but maybe we could
discuss the impact of this proposal. Sure, the above function call in
filterproposaldensity.cpp should be changed.

I want to make the dimensionSet protected since in my viewpoint it is a
function which the user should not be able to call. Imagine that while he's
doing some serieous estimation he suddenly calls dimensionset. The underlying
pdf-representation (Gaussian,discretepdf) is not adapted while doing this and
furthermore, it is very unclear HOW it should be adapted.

Any comments, suggestions?

Tinne

For more infomation about this bug, visit

Klaas Gadeyne <klaas [dot] gadeyne [..] ...> changed:

What |Removed |Added
--------------------------------------------------------------------------
CC| |klaas [dot] gadeyne [..] ...

--- Comment #1 from Klaas Gadeyne <klaas [dot] gadeyne [..] ...> 2007-12-07 09:43:59 ---
(In reply to comment #0)
> Created an attachment (id=161)
--> (https://svn.fmtc.be/bugzilla/orocos/attachment.cgi?id=161) [details]
> Patch that fixes this for discretepdf
>
> The function DimensionSet only changes the variable _dimension of a pdf, but
> never changes the dimension of a pdf.
> The included patch changes the dimension of a discretepdf, and not only the
> variable _dimension.

patch seems ok. However, this patch raises some additional questions to me.

1/ This resize() is a non-realtime operation. If the maintainer decides to
allow it (see 3/), you should at least add a doxygen comment to warn users that
this is a non-realtime operation.

2/ If your patch is valid, please add a unit test to bfl that only succeeds
using your patch and fails without.

2/ I wonder whether the DimensionSet() call should really be in the public API
of pdf? Can you describe the use case in which you would use the
DimensionSet() call.

Furthermore, while investigating this patch, I noticed the following code in
gaussian.cpp

11978 kgadeyne void
11978 kgadeyne Gaussian::ExpectedValueSet (const ColumnVector& mu)
11978 kgadeyne {
12218 kgadeyne _Mu = mu;
27989 wmeeusse assert(this->DimensionGet() == mu.rows());
11992 kgadeyne if (this->DimensionGet() == 0)
11992 kgadeyne {
11992 kgadeyne this->DimensionSet(mu.rows());
11992 kgadeyne }
11978 kgadeyne }

It seems to me the assert() renders the if statement invalid. Moreover, if it
should be there, I guess it's better places _before_ the assignment.
IIRC, the if statement is in place to be able to write something like

Gaussian g; // without args, dimension = 0
ColumnVector v(5) = 0.0;
g.ExpectedValueSet(v); // Non-realtime operation, change dimension to 5.

With the assert in place, this won't work.

However, _without_ the assert, the following won't raise an error

Gaussian g;
ColumnVector v(2)
SymmetricMatrix s(3);
g.ExpectedValueSet(v);
g.CovarianceSet(s);

which is not good either.

At first sight, this is solved by reordering the statements.