Skip to content

Adds Gaussian Function in maths section #1051

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

Closed
wants to merge 21 commits into from
Closed

Adds Gaussian Function in maths section #1051

wants to merge 21 commits into from

Conversation

QuantumNovice
Copy link
Contributor

No description provided.

@cclauss cclauss changed the title Adde Gaussian Fucntion in maths section Adde Gaussian Function in maths section Jul 20, 2019
@QuantumNovice QuantumNovice changed the title Adde Gaussian Function in maths section Adds Gaussian Function in maths section Jul 20, 2019
@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

This is cool! In this PR, please focus only on adding the Gaussian function. Please drop the .md files that are not directly tied to the Gaussian function.

In general, we have the autogenerated DIRECTORY.md for users to find algorithms so if the .md files do not contain explanations, we would rather not have them.

Awesome that you have added doctests but how about a few more? Both good parameters and bad parameters: Large numbers, negative numbers, strings, lists.

@QuantumNovice
Copy link
Contributor Author

This is cool! In this PR, please focus only on adding the Gaussian function. Please drop the .md files that are not directly tied to the Gaussian function.

In general, we have the autogenerated DIRECTORY.md for users to find algorithms so if the .md files do not contain explanations, we would rather not have them.

Awesome that you have added doctests but how about a few more? Both good parameters and bad parameters: Large numbers, negative numbers, strings, lists.
Thanks for consideration. On it.

@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

All submissions will be tested with mypy so we encourage to add Python type hints where it makes sense to do so.

@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

OPTIONAL: Extra credit if you run your code through python/black for readability.

OPTIONAL: You can also try to minimize parens:

    return 1 / sqrt(2 * pi * sigma ** 2) * exp(-(x - mu) ** 2 / 2 * sigma ** 2)

@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

Lowercase directory and Python filenames please:

  • Maths/Gaussian.py --> maths/gaussian.py

@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

@QuantumNovice
Copy link
Contributor Author

QuantumNovice commented Jul 20, 2019

The test failed because in Python 3.7.3
The exception message is 'Result too large'
OverflowError: (34, 'Result too large')
where as in python 3.7.1 the exception string is
OverflowError: (34, 'Numerical result out of range')
My fork is too old. I am updating my fork to the latest commit. I'll take care of that.

@cclauss
Copy link
Member

cclauss commented Jul 20, 2019

OverflowError: (34, ...)

@QuantumNovice
Copy link
Contributor Author

Fixed the error with +IGNORE_EXCEPTION_DETAIL
New Pull

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.

2 participants