-
Notifications
You must be signed in to change notification settings - Fork 31
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
Memoize fibonacci #187
Memoize fibonacci #187
Conversation
945739b
to
d6477aa
Compare
* `fibMemo n = memoize fibFast n` | ||
* `fibMemo = memoize fibFast` | ||
* Move to a `let` or `where` clause inside of `fibFast`. | ||
-} |
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.
If I'm understanding you correctly, adjusting fibMemo
in the above 3 ways will remove the performance benefit of memoization.
Could we convert those examples to code, add them as statements to main
but leave those statements commented out with a header above explaining why?
For example:
main :: Effect Unit
main = do
log $ i "basic fib result" $ fib 7
...
log $ i "fibSlow result: " $ fibSlow 7
-- These examples type check and seem
-- to incur the performance benefits of memoization.
-- However, they do not. Uncomment them to see for yourself.
-- log $ i "fibBroken1 result: " $ fibBroken1 7
-- ..
I think I'd also prefer that main
be immediately below the import statements, so that one can see what will run and then see how they are defined.
Thoughts?
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.
ping @milesfrain
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.
Yeah. These are good suggestions. I'll circle back to this task in about a week. Trying to wrap-up the try-purescript enhancements first.
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.
In that case, I'll push changes to this PR and merge. If you have feedback on what I've done, feel free to open an issue.
Stay focused on the try-purescript stuff as that's more important.
…ken4 fibBroken4 fails to compile. Not sure whether this is a compiler bug or not.
Fixes #182