-
-
Notifications
You must be signed in to change notification settings - Fork 756
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
[WIP] Add Rademacher statistical distribution #245
base: develop
Are you sure you want to change the base?
[WIP] Add Rademacher statistical distribution #245
Conversation
I noticed that the Rademacher distribution does take two parameters like many of the other distributions that are currently in stdlib. Should there be a |
@justin1dennison I am going to defer to the resident distributions expert, @Planeshifter. @Planeshifter How would you parameterize the methods? |
For a distribution without distribution parameters, it doesn't really make sense to offer a factory method, so it's a valid question. Let's not offer a factory method for any of the distribution functions. |
@Planeshifter That seems reasonable to me! |
@kgryte and @Planeshifter thanks for the help. If there isn't an existing implementation in Python, R, or the sort to benchmark, then is the one benchmark the only one required? Or maybe I am just not finding the Rademacher distribution. Thanks again. |
Yes, in cases like this where there are no implementations in the languages we usually compare ourselves to, we just include the JS benchmarks. |
lib/node_modules/@stdlib/stats/base/dists/rademacher/cdf/docs/repl.txt
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/README.md
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/pmf/test/test.js
Outdated
Show resolved
Hide resolved
@justin1dennison This is looking great thus far! Thanks for working on this! |
@kgryte @Planeshifter I noticed that the mean ( as well as median, skewness, etc) for the Rademacher distribution are constant values. I worked on the mean with the assumption that these should still be implemented as functions to maintain the interface like the other distributions. Is that the correct assumption? Additionally, what does that mean for the tests, benchmarks, etc? Thanks. |
Yes, for consistency reasons the distribution properties should still be exposed as functions. Testing would just entail that the package exports a function and that it's return value matches the intended value, with benchmarks being similarly trivial. |
lib/node_modules/@stdlib/stats/base/dists/rademacher/mean/lib/main.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/mean/docs/repl.txt
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/mean/lib/index.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/stats/base/dists/rademacher/mean/benchmark/benchmark.js
Show resolved
Hide resolved
|
||
<!-- <equation class="equation" label="eq:rademacher_median" align="center" raw="\operatorname{Median}\left( X \right) = 0" alt="Median for a Rademacher distribution."> --> | ||
|
||
<div class="equation" align="center" data-raw-text="\operatorname{Median}\left( X \right) = 0" data-equation="eq:bernoulli_median"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<div class="equation" align="center" data-raw-text="\operatorname{Median}\left( X \right) = 0" data-equation="eq:bernoulli_median"> | |
<div class="equation" align="center" data-raw-text="\operatorname{Median}\left( X \right) = 0" data-equation="eq:rademacher_median"> |
Returns the [median][median] of a [Rademacher][rademacher-distribution] distribution. | ||
|
||
```javascript | ||
var v = median( ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var v = median( ); | |
var v = median(); |
var median = require( '@stdlib/stats/base/dists/rademacher/median' ); | ||
``` | ||
|
||
#### median( ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#### median( ) | |
#### median() |
|
||
<section class="links"> | ||
|
||
[rademacher-distribution]: https://en.wikipedia.org/wiki/Bernoulli_distribution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This URL will need updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sure will. 😄
b.tic(); | ||
for ( i = 0; i < b.iterations; i++ ) { | ||
// NOTE: certain compiler optimizations may render this benchmark meaningless... | ||
y = median( ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
y = median( ); | |
y = median(); |
lib/node_modules/@stdlib/stats/base/dists/rademacher/median/docs/repl.txt
Outdated
Show resolved
Hide resolved
1283872
to
b7a0a28
Compare
@Planeshifter What needs to be done in order to get this PR merged? |
Resolves #182 .
Checklist
develop
.develop
branch.Description
This pull request:
Related Issues
This pull request:
Questions
No.
Other
No.
@stdlib-js/reviewers