Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Minor optimizations for nth_fibonacci_number, PEP8 formatting. #2

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

Merged
MTrajK merged 6 commits into MTrajK:master from philippeitis:patch-1
Dec 17, 2019
Merged

Minor optimizations for nth_fibonacci_number, PEP8 formatting. #2

MTrajK merged 6 commits into MTrajK:master from philippeitis:patch-1
Dec 17, 2019

Conversation

@philippeitis
Copy link
Contributor

@philippeitis philippeitis commented Dec 16, 2019

Changed

 dp = [0 for i in range(max(2, n + 1))]

to

 dp = [0] * max(2, n+1)

and cached matrix elements for multiplication to avoid excessive fetching of elements.
Also, a bit of pep8 styling.

Copy link
Owner

MTrajK commented Dec 17, 2019

Hi, the array initialization and brackets changes are nice.
But about the matrix multiplication, that's a thing of taste, I think the first implementation is more readable and intuitive. Do you have any strong opinions why the matrix multiplication should be changed?

sthagen reacted with thumbs up emoji

Copy link
Contributor Author

Hi, the array initialization and brackets changes are nice.
But about the matrix multiplication, that's a thing of taste, I think the first implementation is more readable and intuitive. Do you have any strong opinions why the matrix multiplication should be changed?

That's fair - it's mostly a literal optimization, as fetching a[0][0] multiple times creates a lot of overhead - grabbing the value once is faster.

For 100000 runs

OPT FIB: 0.527823s
UNOPT FIB: 0.579806s

I am aware that this is unlikely to be used in performance demanding situations, but I personally think it's a good idea to practice writing, and especially for beginners, show what optimal code looks like, if it doesn't significantly impact code usability.

You could also take the optimization one step further, and read the values of b once into temporary variables, which yields more significant differences:

OPT FIB: 0.440688s
UNOPT FIB: 0.584284s

I've decided to add a slightly more elegant solution which I think looks a bit nicer, and added an explanation as to how it works to be slightly educational about python's various features.

Copy link
Contributor Author

I also thought it would be nice to see how long each method takes, so I added some comparisons for the 25th Fibonacci number across all seven methods.

Copy link
Owner

MTrajK commented Dec 17, 2019

Great, this one is better, I'll merge it. But first, please restore the "testing" part, we need to follow the template.

philippeitis reacted with thumbs up emoji

Copy link
Contributor Author

The template doesn't specify anything for comparisons such as timing, so I'll let you decide how you want to handle that.

sthagen reacted with thumbs up emoji

Copy link
Owner

MTrajK commented Dec 17, 2019

Thanks, I'll merge and delete the timing comparisons.
The point of the project is to compare only the Time Complexities (Python is used because it is simple and the most understandable language, so it should be easier for someone from another environment to implement the same solution).

sthagen reacted with thumbs up emoji

@MTrajK MTrajK merged commit 500e0bd into MTrajK:master Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /