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

Add recursive fibonacci benchmark #924

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

devyn
Copy link
Contributor

@devyn devyn commented Jul 28, 2024

This is pretty much the least efficient reasonably simple way to implement fibonacci in Nushell, and it's a good benchmark to profile our custom command call performance, as this is the majority of the work being done.

I think it may actually be good to add this and the other one to cargo bench so we can track it over time, but it's also useful as a script so it can easily be profiled.

@fdncred fdncred merged commit 6d6a157 into nushell:main Jul 28, 2024
1 check passed
@fdncred
Copy link
Collaborator

fdncred commented Jul 28, 2024

Thanks. There's also a --pretty on bench that summarizes the output.

@amtoine
Copy link
Member

amtoine commented Jul 28, 2024

is my implementation faster? 😊

let PHI = (1 + (5 | math sqrt)) / 2  # cannot make that a true constant unfortunately

export def fibo [n: int]: [ nothing -> int ] {
    ($PHI ** $n - (1 - $PHI) ** $n) / (5 | math sqrt) | math round --precision 0
}

@fdncred
Copy link
Collaborator

fdncred commented Jul 28, 2024

We could make the math commands const if that's something people are interested in. I just did math sqrt for funsies.

 const PHI = (1 + (5 | math sqrt)) / 2
 $PHI
1.618033988749895

There's also PHI defined in the std lib although I always forget how to use it.

 use std math PHI
 $PHI
1.618033988749895

@devyn devyn deleted the add-recursive-fibonacci branch July 29, 2024 01:27
@devyn
Copy link
Contributor Author

devyn commented Jul 29, 2024

I'm sure it is, though finding the fastest way to do fib isn't really the goal here 😉

@amtoine
Copy link
Member

amtoine commented Jul 29, 2024

@fdncred honestly, i think this is a good idea, solely to be able to define mathematical constants as you did 👍

@devyn
Copy link
Contributor Author

devyn commented Jul 30, 2024

Yeah Rust doesn't allow floating point math in constants for various reasons (external floating point state I think being a major one), but we don't really have that problem, it should be fine for us

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.

3 participants