proposal for new pdf: uniform

Hi,

in attachment you can find a patch for the current trunk, which proposes a new
functionality: i.e. a uniform distribution.

Any comments are welcome,

Tinne

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

proposal for new pdf: uniform

On Monday 17 December 2007 15:59:01 Tinne De Laet wrote:
> Hi,
>
> in attachment you can find a patch for the current trunk, which proposes a
> new functionality: i.e. a uniform distribution.
>
> Any comments are welcome,
>
> Tinne
>
the attachement .. (oeps)

Tinne

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

proposal for new pdf: uniform

On Monday 17 December 2007 16:53:06 Tinne De Laet wrote:
> On Monday 17 December 2007 15:59:01 Tinne De Laet wrote:
> > Hi,
> >
> > in attachment you can find a patch for the current trunk, which proposes
> > a new functionality: i.e. a uniform distribution.
> >
> > Any comments are welcome,
> >
> > Tinne
>
> the attachement .. (oeps)

Apparently something is automatically removing the attachment.
You can also find the attachment at:
http://people.mech.kuleuven.be/~tdelaet/patch_uniform

Tinne
_______________________________________________
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 468] New: proposal for new pdf: uniform

For more infomation about this bug, visit
Summary: proposal for new pdf: uniform
Product: BFL
Version: unspecified
Platform: All
OS/Version: All
Status: NEW
Severity: enhancement
Priority: P2
Component: core
AssignedTo: bfl [..] ...
ReportedBy: tinne [dot] delaet [..] ...
CC: bfl [..] ...
Estimated Hours: 0.0

I propose to introduce some new functionality for a uniform distribution.

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

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

What |Removed |Added
--------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED

--- Comment #7 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-20 09:38:15 ---
I applied the patch (and included some comments in the ChangeLog).

Sending ChangeLog
Sending src/pdf/CMakeLists.txt
Adding src/pdf/uniform.cpp
Adding src/pdf/uniform.h
Sending tests/pdf_test.cpp
Sending tests/pdf_test.hpp
Transmitting file data ......
Committed revision 28794.

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

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

What |Removed |Added
--------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|bfl [..] ... |tinne [dot] delaet [..] ...en.b
| |e

--- Comment #6 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-20 09:34:19 ---
Created an attachment (id=171)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=171)
Uniform pdf patch - bis 2

I created a seperate bug/enhancement for rng (to add a sampler for a uniform
distribution with lower and upper border given).
The patch which is left to apply is attached.

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

--- Comment #5 from Klaas Gadeyne <klaas [dot] gadeyne [..] ...> 2007-12-19 15:26:51 ---
(In reply to comment #4)
> I followed some suggestions of Klaas (I store the lower and higher border of
> the uniform distribution in stead of the center and the width.). (see uniform
> pdf patch-bis)

ACK. Code readability has improved to (for my twisted brain, at least :-)

> Furthermore a adapted the rng wrapper such that samples from a uniform
> distribution with given lower and higher border can be obtained (and not only
> from (0,1)).

I notice you performed a rather radical change here.
luckily this sleeping dog just woke up :-))

You removed the static RNG's and create new ones for _every_ sample generation.
I'm not sure, but I would suspect this will result in an enormous performance
penalty: Everytime you sample, you will create a new RNG on the stack. Did
you check this?

Is this intented? (Even _if_ we do this, I think it would be worth a separate
feature/bug report.

> Last but not least, I also added a unit-test that tests whether the obtained
> samples are located in the area defined by the uniform distribution.
> Again,
> any comments/suggestions are welcome.

woef woef (dog speak for "great work!")

Klaas

(ps. I guess you can click on "accept bug" next time :-)

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

--- Comment #4 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-19 15:03:09 ---
I followed some suggestions of Klaas (I store the lower and higher border of
the uniform distribution in stead of the center and the width.). (see uniform
pdf patch-bis)
Furthermore a adapted the rng wrapper such that samples from a uniform
distribution with given lower and higher border can be obtained (and not only
from (0,1)).
Last but not least, I also added a unit-test that tests whether the obtained
samples are located in the area defined by the uniform distribution.
Again,
any comments/suggestions are welcome.

Tinne

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

--- Comment #3 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-19 15:02:17 ---
Created an attachment (id=167)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=167)
Uniform pdf patch - bis

[Bug 468] proposal for new pdf: uniform

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-19 10:13:48 ---
The BFL community is very grateful for your patch! ;-)

Some comments/questions (forgive my ignorance, I only had a very quick glance)
- Have you thought about alternative implementation schemes and their
advantages/drawbacks? One way that jumps to my mind is to store begin and end
vector and height, instead of center and width. At first sight I think that
would make some implementation faster, e.g. the probabilibyGet call. It would
also allow you to use the rnorm function using rnorm(beginning,end).

- The probget function()
+ Probability Uniform::ProbabilityGet(const ColumnVector& input) const
+ {
+ // test if input is located in area of Uniform distribution
+ bool in_area = 1;
+ for (int i = 1; i < input.rows()+1 ; i++)
+ {
+ double diff = std::abs(input(i) - _Center(i));
+ if (in_area & ( diff<_Width(i)/2 ) )
+ {
+ in_area = 1;
+ }
+ else
+ {
+ in_area = 0;
+ }
+ }
+ if (in_area)
+ {
+ return 1/_Area;
+ }
+ else
+ {
+ return 0;
+ }
+ }

See note above on different implementation scheme. But even if you decide to
keep that scheme, I would suggest to use a while loop for your implementation
for 2 reasons:
- I had to study your code 10 minutes before understanding that this would
actually work :-)
- A while loop is more efficient, as you don't have to loop over the whole
dimension of the random variable. As soon as you detect the variable is not
situated "in_area", you can break out of the loop.

ps. It also seems that your patch contains all diff twice (something in my
brain tells me I've seen that issue before)

Klaas

[Bug 468] proposal for new pdf: uniform

For more infomation about this bug, visit

--- Comment #1 from Tinne De Laet <tinne [dot] delaet [..] ...> 2007-12-18 17:42:50 ---
Created an attachment (id=166)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=166)
Uniform pdf patch

Uniform pdf functionality.