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

Lao polynomial equation issues in equilibria.rst and implementation #3625

Closed
timothy-nunn opened this issue Oct 1, 2024 · 1 comment · Fixed by #3634
Closed

Lao polynomial equation issues in equilibria.rst and implementation #3625

timothy-nunn opened this issue Oct 1, 2024 · 1 comment · Fixed by #3634
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@timothy-nunn
Copy link
Collaborator

timothy-nunn commented Oct 1, 2024

Describe the bug

Documentation issue

The equation (14) in equilibria.rst provides the Lao polynomial parameterisation for the plasma profiles.

The equation, however, disagrees with Lao et al. [1] and the docstring in bluemira/equilibria/profiles/laopoly. I believe that the equation should instead read:
$g\big(\overline{\psi},\boldsymbol{\alpha}\big) = \sum_{n=0}^{N}\alpha_{n+1}\overline{\psi}^{~n}-\overline{\psi}^{~N+1}\sum_{n=0}^{N} \alpha_{n+1}$.

That is, the first normalised $\psi$ should be to the power of $n$, not $n+1$.

However, rewriting it in keeping with Lao et al. may reduce confusion further:
$g\big(\overline{\psi},\boldsymbol{\alpha}\big) = \sum_{n=0}^{N}\alpha_{n}\overline{\psi}^{~n}-\overline{\psi}^{~N+1}\sum_{n=0}^{N} \alpha_{n}$.
Where $N$ is the number of coefficients -1: $N = |\alpha| -1$.

e.g. for $\alpha=[1, 2]$, $N=2-1=1$ and the equation becomes $g(x) = (1x^0 + 2x^1) - x^2(1+2)$

Implementation issue

The final term of bluemira/equilibria/profiles/laopoly raises the normalised equilibrium to N+2 (see the above definition) but I believe it should be to N+1.

Because N is defined as the number of constants minus one, this final term of the equation raises the normalised psi to the power of the number of alpha constants. However, the implementation does x ** (len(args) + 1) which is essentially raising it to $N+1 +1$.

[1] Lao, L. L., et al. "Reconstruction of current profile parameters and plasma shapes in tokamaks." Nuclear fusion 25.11 (1985): 1611.

@timothy-nunn timothy-nunn added the documentation Improvements or additions to documentation label Oct 1, 2024
@timothy-nunn timothy-nunn changed the title Lao polynomial equation incorrect in equilibria.rst Lao polynomial equation issues in equilibria.rst and implementation Oct 4, 2024
@OceanNuclear
Copy link
Contributor

After a detailed check of the maths, I can confirm that this is indeed a mistake.
I have plotted
However, the plot belowImage
, generated using the data @timothy-nunn gave me, unfortunately happens to use data whose final term (the term mistakenly raised to $N+1+1$) happen to have coefficient sum(args)$=0$, so the error is invisible in this case.

Regardless, I am pretty confident that @timothy-nunn is right, so I have implemented the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants