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
Improve Project Euler problem 014 solution 2 #5744
Improve Project Euler problem 014 solution 2 #5744
Conversation
Nice work overall. You replaced the loop with recursion, possibly making the code faster. Also, having the sequence lengths as a global dictionary would save a lot of time computing.
Maybe note such changes in the commit message next time (E.g. loop to recursion).
Sincerely,
IdoErel
| sequence_length = collatz_sequence_length(next_n) + 1 | ||
| COLLATZ_SEQUENCE_LENGTHS[n] = sequence_length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary variable assignment.
This can be shortened to one line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then we should return COLLATZ_SEQUENCE_LENGTHS[n] instead of sequence_length. I don't know Python perfectly - whether there will be an extra search in the dictionary or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you're right. I missed line 43.
Disregard my previous comment.
I originally meant, that assigning the result to sequence_length - and immediately after assigning it again to COLLATZ_SEQUENCE_LENGTHS, without any further usage is unnecessary. (of sequence_length).
However, you're returning sequence_length as the result. Saving another lookup - in short, keep it this way. I was wrong.
Sorry for my ignorance.
Sincerely,
IdoErel

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Describe your change:
Improve Project Euler problem 014 solution 2 - the top 1 slowest solution on Travis CI logs (under
slowest 10 durations:14.20s call scripts/validate_solutions.py::test_project_euler[problem_014/sol2.py]):Improve solution (locally 10+ times - from 15+ seconds to ~1.5 seconds)
Uncomment code that has been commented due to slow execution affecting Travis (now it should be quite fast execution and not affect Travis)
Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?
Checklist:
Fixes: #{$ISSUE_NO}.