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

Tribonacci #1398

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Tribonacci #1398

wants to merge 7 commits into from

Conversation

raoulb
Copy link

@raoulb raoulb commented May 26, 2023

Carry over an old PR from Calcium to Flint

flintlib/calcium#42

@raoulb
Copy link
Author

raoulb commented Jun 29, 2023

@fredrik-johansson Please consider this series of PRs for merging or let me know what is missing. May be this can even go into flint 3?

@videlec
Copy link
Contributor

videlec commented Oct 16, 2023

Wouldn't it be nicer with a generic k-bonacci x^k - x^(k-1) - x^(k-2) - ... - 1 that specializes to Goldenratio (Fibonacci), Tribonacci and Tetranacci?

This was referenced Oct 16, 2023
@raoulb
Copy link
Author

raoulb commented Oct 28, 2023

Hmm, I'm not sure I get your point. Which part of the code do you talk about?

Sure, there could be a function which constructs the general polynomial. But this series of PRs implements the constants, which are selected solutions of the corresponding polynomials. (And there even is no closed-form solution for degree 5 up to at least 11, according the the cited reference.)

Maybe we could write (for example) a single qqbar_kbonacci_constant(qqbar_t res, uint k) function, but I don't see the real benefit, because the main part of each individual function is constructing the expression for the explicit solution.

If this is about the example programs, then, yes, we could merge them or even just include a single one, if at all. They are my test cases used for development.

Maybe I miss the obvious point here. In that case, please give more details about what I should change in the code. Thanks.

@videlec
Copy link
Contributor

videlec commented Oct 29, 2023

Hmm, I'm not sure I get your point. Which part of the code do you talk about?

Sure, there could be a function which constructs the general polynomial. But this series of PRs implements the constants, which are selected solutions of the corresponding polynomials. (And there even is no closed-form solution for degree 5 up to at least 11, according the the cited reference.)

Maybe we could write (for example) a single qqbar_kbonacci_constant(qqbar_t res, uint k) function, but I don't see the real benefit, because the main part of each individual function is constructing the expression for the explicit solution.

If this is about the example programs, then, yes, we could merge them or even just include a single one, if at all. They are my test cases used for development.

Maybe I miss the obvious point here. In that case, please give more details about what I should change in the code. Thanks.

By constructing the expression for the explicit solution, you mean expressing the k-bonacci in terms of n-th roots? Isn't the function qqbar_get_fexpr_formula supposed to (partially) do that already? There is a general algorithm for writing such expressions for any algebraic number, see eg in sagemath

sage: x = polygen(QQ, 'x')
sage: tribonacci = QQbar.polynomial_root(x^3 - x^2 - x - 1, RIF(1, 2))
sage: tribonacci.radical_expression()
1/3*(3*sqrt(11)*sqrt(3) + 19)^(1/3) + 4/3/(3*sqrt(11)*sqrt(3) + 19)^(1/3) + 1/3

The above is, as far as I know, not fully implemented in flint but would be to my mind a better addition. While it is valuable to have concrete examples for illustration and testing purposes, I do not understand what would be the advantage of having these symbolic expressions hard coded for each single constant that someone care about.

Note that this is my personal opinion. It does not necessarily reflect the point of view of the of the flint maintainer team.

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