Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ampersand Remove state for DiscretePdf #34

Open
toeklk opened this issue Dec 9, 2017 · 0 comments
Open

Add ampersand Remove state for DiscretePdf #34

toeklk opened this issue Dec 9, 2017 · 0 comments

Comments

@toeklk
Copy link
Collaborator

toeklk commented Dec 9, 2017

migrated from Bugzilla #473
status ASSIGNED severity enhancement in component core for ---
Reported in version trunk on platform All
Assigned to: François Cauwe

Original attachment names and IDs:

On 2007-12-21 00:21:31 +0100, François Cauwe wrote:

  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

On 2007-12-21 00:23:43 +0100, François Cauwe wrote:

  Created attachment 174 Patch This patches adds this functionality, and also updates the unit tests.

On 2007-12-21 08:49:10 +0100, Klaas Gadeyne wrote:

  (In reply to comment # 1) > Created an attachment (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!

On 2007-12-22 00:16:05 +0100, François Cauwe wrote:

  Created attachment 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

On 2008-01-13 18:22:19 +0100, François Cauwe wrote:

Are there still some comments on this patch?

On 2008-01-16 10:30:52 +0100, Tinne De Laet wrote:

  Francois, You should update your patch according to the latest changes to the trunk (see bug # 475.) Tinne
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant