feat: added stats/base/dists/boltzmann/pmf #2108
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kgryte
added
Feature
kgryte
reviewed
Apr 2, 2024
Comment on lines
+29
to
+33
| ```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.
This isn't how we insert equations. You only need to include the HTML comment and then we have build tooling which auto-inserts.
kgryte
reviewed
Apr 2, 2024
| // 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.
Suggested change
| 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`. |
kgryte
reviewed
Apr 2, 2024
| // returns NaN | ||
| ``` | ||
|
|
||
| If provided inverse temperature parameter `λ` is not a positive number, the function returns `NaN`. |
There was a problem hiding this comment.
Suggested change
| If provided inverse temperature parameter `λ` is not a positive number, the function returns `NaN`. | |
| If `λ` is not a positive number, the function returns `NaN`. |
kgryte
reviewed
Apr 2, 2024
|
|
||
| #### 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.
Suggested change
| 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 `λ`. |
kgryte
reviewed
Apr 2, 2024
| var i; | ||
|
|
||
| for ( i = 0; i < 10; i++ ) { | ||
| N = round( randu() * 20 ); |
There was a problem hiding this comment.
Use discrete-uniform rather than round and randu.
kgryte
reviewed
Apr 2, 2024
| {{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.
Suggested change
| temperature analysis `λ` at a value `x`. | |
| temperature parameter `λ` at a value `x`. |
kgryte
reviewed
Apr 2, 2024
|
|
||
| If provided `NaN` as any argument, the function returns `NaN`. | ||
|
|
||
| If provided total number of energy states `N`, inverse temperature |
kgryte
reviewed
Apr 2, 2024
|
|
||
| {{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.
Suggested change
| boltzmann distribution with total number of energy states `N`, and | |
| Boltzmann distribution with total number of energy states `N` and |
kgryte
reviewed
Apr 2, 2024
| // TypeScript Version: 4.1 | ||
|
|
||
| /** | ||
| * Evaluates the probability mass function (PMF) for a boltzmann distribution. |
There was a problem hiding this comment.
Suggested change
| * 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.
kgryte
reviewed
Apr 2, 2024
Comment on lines
+36
to
+39
| * ## 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.
Suggested change
| * ## 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.
kgryte
reviewed
Apr 2, 2024
| */ | ||
| 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.
Suggested change
| * 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 `λ`. |
kgryte
reviewed
Apr 2, 2024
| * Boltzmann distribution probability mass function (PMF). | ||
| * | ||
| * @param x - input value | ||
| * @param N - total states of energy |
There was a problem hiding this comment.
Suggested change
| * @param N - total states of energy | |
| * @param N - total number of energy states |
Be consistent in your descriptions.
kgryte
reviewed
Apr 2, 2024
| 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.
Suggested change
| // 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... |
kgryte
reviewed
Apr 2, 2024
| 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.
Suggested change
| // 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.. |
kgryte
reviewed
Apr 2, 2024
|
|
||
| for ( i = 0; i < 10; i++ ) { | ||
| lambda = randu(); | ||
| N = round( randu() * 20.0 ); |
kgryte
reviewed
Apr 2, 2024
| // 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.
Suggested change
| * 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.
kgryte
reviewed
Apr 2, 2024
Comment on lines
+36
to
+37
| * @param {PositiveNumber} lambda - Inverse Temperature | ||
| * @param {NonNegativeInteger} N - Total Number of energy states |
There was a problem hiding this comment.
Suggested change
| * @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.
kgryte
reviewed
Apr 2, 2024
| isnan( N ) || | ||
| isnan( lambda ) || | ||
| !isNonNegativeInteger( N ) || | ||
| !isNonNegativeFinite( lambda ) || |
kgryte
reviewed
Apr 2, 2024
| isnan( lambda ) || | ||
| !isNonNegativeInteger( N ) || | ||
| !isNonNegativeFinite( lambda ) || | ||
| N === PINF || |
There was a problem hiding this comment.
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.
kgryte
reviewed
Apr 2, 2024
| return pmf; | ||
|
|
||
| /** | ||
| * Evaluates the probability mass function (PMF) for a Boltzmann distribution. |
There was a problem hiding this comment.
This is not how we indent JSDoc comments.
kgryte
reviewed
Apr 2, 2024
| } | ||
| if ( | ||
| isNonNegativeInteger( x ) && | ||
| x < N |
kgryte
reviewed
Apr 2, 2024
| } | ||
| if ( | ||
| isNonNegativeInteger( x ) && | ||
| x < N |
kgryte
reviewed
Apr 2, 2024
| N = round.( ( rand( 100 ) ) .+ 50 ); | ||
| K = round.( rand( 1000 ) .* 10 ); | ||
| x = round.( rand( 1000 ) .* N ); | ||
| gen( x, N, K, n, "data.json" ); |
kgryte
reviewed
Apr 2, 2024
| # 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.
There is no Boltzmann distribution in Julia distributions tmk: https://juliastats.org/Distributions.jl/stable/search/?q=boltzmann
You should probably test against SciPy.
kgryte
reviewed
Apr 2, 2024
| 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.
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. |
kgryte
requested changes
Apr 2, 2024
There was a problem hiding this comment.
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.
kgryte
added
the
Statistics
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
kgryte
added
the
Needs Changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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