[Bug 473] New: Add & Remove state for DiscretePdf

For more infomation about this bug, visit
Summary: Add & Remove state for DiscretePdf
Product: BFL
Version: trunk
Platform: All
OS/Version: All
Status: NEW
Severity: enhancement
Priority: P2
Component: core
AssignedTo: bfl [..] ...
ReportedBy: francois [..] ...
CC: bfl [..] ...
Estimated Hours: 0.0

Add the possibility to add and remove a component in discretePdf.

This is needed for the Mixture of Gaussian as discussed in:
http://lists.mech.kuleuven.be/pipermail/bfl/2007-December/000822.html

[Bug 473] Add & Remove state for DiscretePdf

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 #5 from Tinne De Laet <tinne [dot] delaet [..] ...> 2008-01-16 10:30:52 ---
Francois,

You should update your patch according to the latest changes to the trunk (see
bug #475.)

Tinne

[Bug 473] Add & Remove state for DiscretePdf

For more infomation about this bug, visit

--- Comment #4 from François Cauwe <francois [..] ...> 2008-01-13 18:22:19 ---
Are there still some comments on this patch?

[Bug 473] Add & Remove state for DiscretePdf

For more infomation about this bug, visit

François Cauwe <francois [..] ...> changed:

What |Removed |Added
--------------------------------------------------------------------------
Attachment #174 is|0 |1
obsolete| |

--- Comment #3 from François Cauwe <francois [..] ...> 2007-12-22 00:16:05 ---
Created an attachment (id=176)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=176)
Updated patch

Thanks for the feedback, I made a new patch and:
* Added a Changelog message [1]
* Make a better unit test, checked all states.
* Change the variable names, so it won't create any confusion with
probability. See also bug #475

[1] http://lists.mech.kuleuven.be/pipermail/bfl/2007-December/000868.html

[Bug 473] Add & Remove state for DiscretePdf

For more infomation about this bug, visit

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

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

--- Comment #2 from Klaas Gadeyne <klaas [dot] gadeyne [..] ...> 2007-12-21 08:49:10 ---
(In reply to comment #1)
> Created an attachment (id=174)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=174) [details]
> Patch
> This patches adds this functionality, and also updates the unit tests.

Patch seems ok. 2 small remarks:
- Try generating your patches using svn diff -x -u, so they are somewhat more
readable.
- I think you should make the documentation about stateAdd somewhat more
explicit, in casu about the meaning you attach to the parameter "probability
p". As I understand from your patch, this is the probability of the new
discrete state _after_ renormalizing? Anyway, from your doxygen comment, this
is not clear to me.
As an example, suppose I have a discretePdf with
state 0: prob 0.5
state 1: prob 0.5

When I now perform a pdf.stateAdd(0.5), this will result in

state 0: prob 0.25
state 1: prob 0.25
state 2: prob 0.5

However, I could also expect it to be
state 0: prob 0.33
state 1: prob 0.33
state 2: prob 0.33

You should be more explicit in the docs about this behaviour I guess.
Thx for your contribution!

[Bug 473] Add & Remove state for DiscretePdf

[...]
> Patch seems ok. 2 small remarks:
> - Try generating your patches using svn diff -x -u, so they are somewhat
> more readable.
> - I think you should make the documentation about stateAdd somewhat more
> explicit, in casu about the meaning you attach to the parameter
> "probability p". As I understand from your patch, this is the probability
> of the new discrete state _after_ renormalizing? Anyway, from your doxygen
> comment, this is not clear to me.
>
> You should be more explicit in the docs about this behaviour I guess.
I agree with Klaas. It is very important, certainly in this case, to describe
what your functions are doing. For adding and removing a state there are many
ways in which the probabilities could change, so please describe carefully!
Furthermore, I think you should add some extra tests to the unit tests, to
check if the probabilities of all states (and not only the one added) changed
as expected (they should still be normalized..).
(so add unit tests to test all probabilities after you added or removed a
state.).

Keep on the good work!

Tinne

PS: Could you also add some lines in the Changelog describing your
enhancement?

Disclaimer: http://www.kuleuven.be/cwis/email_disclaimer.htm

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

Disclaimer: http://www.kuleuven.be/cwis/email_disclaimer.htm

[Bug 473] Add & Remove state for DiscretePdf

For more infomation about this bug, visit

François Cauwe <francois [..] ...> changed:

What |Removed |Added
--------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|bfl [..] ... |francois [..] ...

--- Comment #1 from François Cauwe <francois [..] ...> 2007-12-21 00:23:43 ---
Created an attachment (id=174)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=174)
Patch

This patches adds this functionality, and also updates the unit tests.