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

Use Dijkstra identities for 0(0th) Fibonacci convention #1001

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

ryanelandt
Copy link
Contributor

Dijkstra's note "In honour of Fibonacci" provides identities for the convention where 0 is the 1st Fibonacci number. Boost Math uses the convention that 0 is the 0th Fibonacci number. This PR updates these identities to follow the 0 is the 0th convention.

@NAThompson
Copy link
Collaborator

But this is merely convention no?

@ryanelandt
Copy link
Contributor Author

The identity depends on the convention.

Using the 0(0th) convention the first formula is: F(2n-1) = F(n-1)^2 + F(n)^2
Using the 0(1st) convention the first formula is: F(2n) = F(n)^2 + F(n+1)^2

To see why, let's assign a letter to the Fibonacci numbers as follows: a=0, b=1, c=1, d=2, e=3, f=5, g=8, h=13, ... . Now consider the formula:
d^2 + e^2 = h

Using the 0(0th) convention d=3rd, e=4th, h=7th, so the 0(0th) formula is right, but the 0(1st) formula is wrong.
Using the 0(1st) convention d=4th, e=5th, h=8th, so the 0(0th) formula is wrong, but the 0(1st) formula is right.

@NAThompson
Copy link
Collaborator

But this only changes a comment, so the code is ostensibly still correct?

@ryanelandt
Copy link
Contributor Author

ryanelandt commented Jul 25, 2023

The code is correct. But, for comments to be sensible, they need to explain what the code is doing. The original comments do not do this. Consider an analogous situation...

The commented function below converts temperature:

double kelvin_to_rankine(double temp_in) {
  // Uses the formula temp_out = 9/5 * temp_in + 32
  return temp_in * 1.8;
}

The comment explaining what this function does is not sensible because the formula is wrong, it's the formula for something else. Instead the commented function should read:

double kelvin_to_rankine(double temp_in) {
  // Uses the formula temp_out = 9/5 * temp_in as both kelvin and rankine are zero at absolute zero
  return temp_in * 1.8;
}

In the Fibonacci case here, the formulas in the comments need to match the convention being used otherwise they are confusing as they return incorrect outputs.

@NAThompson NAThompson merged commit 281f491 into boostorg:develop Jul 25, 2023
56 checks passed
@NAThompson
Copy link
Collaborator

@ryanelandt : You've convinced me-merged.

Build failure is a timeout.

@ryanelandt
Copy link
Contributor Author

Awesome, thanks.

@ryanelandt ryanelandt deleted the fibonacci_comment_shift branch July 25, 2023 17:58
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