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

remove unnecessary var from numerator and denominator in geometic equation and tidy up /algorithm code #281

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

albertstill
Copy link
Contributor

There is an unnecessary var being used in the geometric equation, I've removed it and tidied up the code. Note there are tests to cover the geometric equation, them passing helps prove it's the same equation with this var removed.

Here is some handwritten maths explaining why the max need not live at the top and bottom of the fraction, it just cancels out. I start with what we used to have and finish with what this PR changes to.

IMG_20200521_171308

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Seems reasonable

@albertstill
Copy link
Contributor Author

Can we merge this now?

@albertstill
Copy link
Contributor Author

What about now?

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