-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
feat: added stats/base/dists/boltzmann/pmf #2108
base: develop
Are you sure you want to change the base?
feat: added stats/base/dists/boltzmann/pmf #2108
Conversation
```math | ||
f(x; \lambda, N) = P(X=x; \lambda, N) = \begin{cases} | ||
\frac{{(\exp(-\lambda k))}{(1-\exp(-\lambda))}}{{(1-\exp(-\lambda N))}} & \text{for } k = 0, 1, 2, \ldots, N \\0 & \text{otherwise} | ||
\end{cases} | ||
``` |
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 isn't how we insert equations. You only need to include the HTML comment and then we have build tooling which auto-inserts.
// returns NaN | ||
``` | ||
|
||
If provided total number if energy states `N` is not a nonnegative integer, the function returns `NaN`. |
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.
If provided total number if energy states `N` is not a nonnegative integer, the function returns `NaN`. | |
If `N` is not a nonnegative integer, the function returns `NaN`. |
// returns NaN | ||
``` | ||
|
||
If provided inverse temperature parameter `λ` is not a positive number, the function returns `NaN`. |
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.
If provided inverse temperature parameter `λ` is not a positive number, the function returns `NaN`. | |
If `λ` is not a positive number, the function returns `NaN`. |
|
||
#### pmf.factory( λ, N ) | ||
|
||
Returns a function for evaluating the [probability mass function][pmf] (PMF) of a [boltzmann][boltzmann-distribution] distribution with total number of energy states `N` and inverse Temperature `λ`. |
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.
Returns a function for evaluating the [probability mass function][pmf] (PMF) of a [boltzmann][boltzmann-distribution] distribution with total number of energy states `N` and inverse Temperature `λ`. | |
Returns a function for evaluating the [probability mass function][pmf] (PMF) of a [boltzmann][boltzmann-distribution] distribution with total number of energy states `N` and inverse temperature `λ`. |
var i; | ||
|
||
for ( i = 0; i < 10; i++ ) { | ||
N = round( randu() * 20 ); |
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.
Use discrete-uniform rather than round
and randu
.
{{alias}}( x, λ, N ) | ||
Evaluates the probability mass function (PMF) for a boltzmann | ||
distribution with total number of energy states `N` and inverse | ||
temperature analysis `λ` at a value `x`. |
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.
temperature analysis `λ` at a value `x`. | |
temperature parameter `λ` at a value `x`. |
|
||
If provided `NaN` as any argument, the function returns `NaN`. | ||
|
||
If provided total number of energy states `N`, inverse temperature |
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 sentence is not correct.
|
||
{{alias}}.factory( λ, N ) | ||
Returns a function for evaluating the probability mass function (PMF) of a | ||
boltzmann distribution with total number of energy states `N`, and |
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.
boltzmann distribution with total number of energy states `N`, and | |
Boltzmann distribution with total number of energy states `N` and |
// TypeScript Version: 4.1 | ||
|
||
/** | ||
* Evaluates the probability mass function (PMF) for a boltzmann 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.
* Evaluates the probability mass function (PMF) for a boltzmann distribution. | |
* Evaluates the probability mass function (PMF) for a Boltzmann distribution. |
Boltzmann should be capitalized here and everywhere.
* ## Notes | ||
* | ||
* - If provided a total number of energy states `N` and inverse temperature parameter `λ` which is not a nonnegative integer, the function returns `NaN`. | ||
* |
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.
* ## Notes | |
* | |
* - If provided a total number of energy states `N` and inverse temperature parameter `λ` which is not a nonnegative integer, the function returns `NaN`. | |
* |
Evident from examples.
*/ | ||
interface PMF { | ||
/** | ||
* Evaluates the probability mass function (PMF) for a boltzmann distribution with total number of energy states `N`, and inverse temperature parameter `λ`. |
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.
* Evaluates the probability mass function (PMF) for a boltzmann distribution with total number of energy states `N`, and inverse temperature parameter `λ`. | |
* Evaluates the probability mass function (PMF) for a boltzmann distribution with total number of energy states `N` and inverse temperature parameter `λ`. |
* Boltzmann distribution probability mass function (PMF). | ||
* | ||
* @param x - input value | ||
* @param N - total states of energy |
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.
* @param N - total states of energy | |
* @param N - total number of energy states |
Be consistent in your descriptions.
pmf( 1, 10, 8 ); // $ExpectType number | ||
} | ||
|
||
// The compiler throws an error if the function is provided values other than four numbers... |
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.
// The compiler throws an error if the function is provided values other than four numbers... | |
// The compiler throws an error if the function is provided values other than three numbers... |
fcn( 2, 0, 1 ); // $ExpectError | ||
} | ||
|
||
// The compiler throws an error if the `factory` method is provided values other than three numbers.. |
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.
// The compiler throws an error if the `factory` method is provided values other than three numbers.. | |
// The compiler throws an error if the `factory` method is provided values other than two numbers.. |
|
||
for ( i = 0; i < 10; i++ ) { | ||
lambda = randu(); | ||
N = round( randu() * 20.0 ); |
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.
Same comment as README.
// MAIN // | ||
|
||
/** | ||
* Returns a function for evaluating the probability mass function (PMF) for a Boltzmann distribution with total number of energy states `N`, and the inverse temperature `lambda`. |
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.
* Returns a function for evaluating the probability mass function (PMF) for a Boltzmann distribution with total number of energy states `N`, and the inverse temperature `lambda`. | |
* Returns a function for evaluating the probability mass function (PMF) for a Boltzmann distribution with total number of energy states `N` and the inverse temperature `lambda`. |
This comma is not needed here and in other descriptions. Please remove.
* @param {PositiveNumber} lambda - Inverse Temperature | ||
* @param {NonNegativeInteger} N - Total Number of energy states |
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.
* @param {PositiveNumber} lambda - Inverse Temperature | |
* @param {NonNegativeInteger} N - Total Number of energy states | |
* @param {PositiveNumber} lambda - inverse temperature | |
* @param {NonNegativeInteger} N - total number of energy states |
We don't capitlize.
isnan( N ) || | ||
isnan( lambda ) || | ||
!isNonNegativeInteger( N ) || | ||
!isNonNegativeFinite( lambda ) || |
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 should be isPositiveNumber
isnan( lambda ) || | ||
!isNonNegativeInteger( N ) || | ||
!isNonNegativeFinite( lambda ) || | ||
N === PINF || |
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.
You have mixed indentation in this file. Please ensure that you have run make init
prior to pushing code. Having to point out these things in review is a burden on developers, especially when we've invested significant time in automation to catch these bugs locally.
return pmf; | ||
|
||
/** | ||
* Evaluates the probability mass function (PMF) for a Boltzmann 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 is not how we indent JSDoc comments.
} | ||
if ( | ||
isNonNegativeInteger( x ) && | ||
x < N |
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.
TABS
} | ||
if ( | ||
isNonNegativeInteger( x ) && | ||
x < N |
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.
TABS
N = round.( ( rand( 100 ) ) .+ 50 ); | ||
K = round.( rand( 1000 ) .* 10 ); | ||
x = round.( rand( 1000 ) .* N ); | ||
gen( x, N, K, n, "data.json" ); |
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.
Trailing newline.
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
import Distributions: pmf, Boltzmann |
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.
There is no Boltzmann distribution in Julia distributions tmk: https://juliastats.org/Distributions.jl/stable/search/?q=boltzmann
You should probably test against SciPy.
t.equal( y, expected[i], 'x: '+x[i]+', lambda: '+lambda[i]+', N: '+N[i]+', y: '+y+', expected: '+expected[i] ); | ||
} else { | ||
delta = abs( y - expected[ i ] ); | ||
tol = 1040.0 * EPS * abs( expected[ i ] ); |
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 is a rather large tolerance. Why the discrepancy?
You shouldn't be manually adding test fixtures. You should be using our test fixtures scripts. In this case, you should test against SciPy. See other packages which test against SciPy. |
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.
Thanks for working on this, @AgPriyanshu18. I left an initial round of comments. This PR needs a fair amount of clean up before we can move this forward.
Feat added the package stats/base/dists/boltzmann/pmf Fixes stdlib-js#179
Feat added the package stats/base/dists/boltzmann/pmf Fixes stdlib-js#179
Added the missing x values in dataset.json file Fixes stdlib-js#179
Resolves #179 .
Description
this pull request added the package stats/base/dists/boltzmann/pmf
This pull request:
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers