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

feat(math): Add distribution functions. #2774

Closed
wants to merge 7 commits into from

Conversation

retraigo
Copy link
Contributor

This PR adds two functions under the math module.

uniformRange generates an array of numbers uniformly distributed between a range in ascending order.

normalDistribution generates an array of numbers normally distributed around a mean in random order.

Tests have been written under the distributions_test.ts file.

@CLAassistant
Copy link

CLAassistant commented Oct 13, 2022

CLA assistant check
All committers have signed the CLA.

n: number,
options: RangeOptions = BASE_RANGE_OPTIONS,
): number[] {
if (typeof n !== "number" || n <= 0 || !Number.isInteger(n)) return [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should throw TypeError instead of returning empty array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it throw a TypeError in the latest commit.

@kt3k
Copy link
Member

kt3k commented Oct 24, 2022

@retraigo Do you have any image about the scope of math module? What kind of items do you suggest we should add in math module?

@retraigo
Copy link
Contributor Author

@retraigo Do you have any image about the scope of math module? What kind of items do you suggest we should add in math module?

Finding math-related JS/TS code on the internet is a really tedious process. There are only a few math-related modules like mathjs out there. Having a standard, third-party-dependency-free math module for Deno would be great.

It could support any math-related stuff that makes sense to do in JavaScript like,

  • Range generation (like the python range())
  • ndarrays (a stricter version that involves a private variable maybe?)
  • Gamma
  • Complex numbers
    or even some basic calculus.

But IMO it shouldn't contain math that operates on a large amount of data (like regression analysis, matrices, etc). Not because they're a bad feature, but they'd be making devs choose them over actually performant third-party modules that use native code.

@cjihrig
Copy link
Contributor

cjihrig commented Oct 24, 2022

The CI is looking pretty red after the last set of changes.

Deno.test({
name: "Negative, zero, non-integer value of n returns an empty array.",
fn() {
assertEquals(uniformRange(-10, { min: 1, max: 10 }).length, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this now should use assertThrows to check it throws TypeError

@kt3k
Copy link
Member

kt3k commented Oct 25, 2022

@retraigo Thanks for many suggestions!

ndarrays (a stricter version that involves a private variable maybe?)

Is it like ndarrays in numpy? What does stricter version mean?

Gamma

Can you elaborate? What is the use case of this?

Complex numbers or even some basic calculus.

This sounds like common needs for math calculation to me.

Range generation (like the python range())

There's Number.range proposal (stage 1). Maybe we should follow their API if we add it.

But IMO it shouldn't contain math that operates on a large amount of data (like regression analysis, matrices, etc). Not because they're a bad feature, but they'd be making devs choose them over actually performant third-party modules that use native code.

I agree with this. We shouldn't compete with/re-invent something like tensorflow.js

@retraigo
Copy link
Contributor Author

Is it like ndarrays in numpy? What does stricter version mean?
I meant how you can have ndarrays by just nesting arrays inside each other. A stricter version as in, forcing types and dimensions.

Gamma
Can you elaborate? What is the use case of this?
Complex numbers or even some basic calculus.
This sounds like common needs for math calculation to me.
Maybe they are a bit too much. I was just listing common things people require math libraries for.

There's Number.range proposal (stage 1). Maybe we should follow their API if we add it.
I didn't know a proposal existed for that. It does sound cool.

@kt3k
Copy link
Member

kt3k commented Oct 25, 2022

We discussed this PR internally, and we are worried about the scope of std/math being too vague at this point. We'd appreciate if you create your own math library in deno.land/x as a 3rd party module, and collect some use cases first, but we don't think accepting random items one by one in std/math is a good idea.

@cjihrig
Copy link
Contributor

cjihrig commented Oct 25, 2022

I'm going to close this per #2774 (comment), but please do create a module on deno.land/x and iterate on it. We can revisit this after a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants