-
Notifications
You must be signed in to change notification settings - Fork 69
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
Bring the notation in elliptic_integrals.{hpp,cpp} in line with the papers #2359
Bring the notation in elliptic_integrals.{hpp,cpp} in line with the papers #2359
Conversation
@@ -780,6 +780,48 @@ @TechReport{Westra2017 | |||
month = sep | |||
} | |||
|
|||
@Article{Bulirsch1965, | |||
author="Bulirsch, Roland", |
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.
Could we be consistent in how we name authors? As far as I can tell we generally say "R. Bulirsch".
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 the generated PDF, we do; in the .bib file, it is best to have as much information as possible, in case we want to switch to an unabbreviated style.
Angle EllipticΠ(Angle const& φ, | ||
double n, | ||
double mc); | ||
// Returns the incomplete elliptic integral of the third kind Π(φ, n|m), where |
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.
Often, n appears first in the list of parameters of Π, followed by a semicolon. But the notation varies...
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.
Fukushima uses Π(φ, n|m).
// though. | ||
// This algorithm (from [Fuk18]) differs from the one in [Fuk11a] because it | ||
// divides log(q(mc)), not just log(mc / 16). It tries to retain the same | ||
// notation, though. |
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.
Point at our documentation.
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.
Done.
fukushima_elliptic_Js_maclaurin_n_4_4.Evaluate(n))); | ||
// The first five coefficients Jₗ(n|m); the others are computed as needed. | ||
// TODO(egg): J₁(n|m) = 1/3; could we make it constexpr? | ||
double const J₁ = fukushima_elliptic_Js_maclaurin_m_0.Evaluate(m); |
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.
The change of index is confusing because J1 is for m^0 now. Could we go back to 0-based?
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.
J1 is for y1 in the result; this is 0-based, there just is no constant term.
This is the convention used in the paper.
Your indexing led to -1-based indices in the coefficient of the series.
// TODO(phl): Use a binary split of [0, 1] to reduce the number of | ||
// comparisons. | ||
double const m = 1.0 - mc; | ||
double const m = 1.0 - mʹ; |
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.
I don't think it's useful to swtch from mc
to mʹ
for this function. As I recall this is the only place where Fukushima uses this notation, and the notation is in general messy and inconsistent enough that we shouldn't add to the mess.
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.
We mention K′ below (in the existing comment as well), so we use the notation from [Fuk09] here. If we start mixing notations between papers we are going to get a mess: calling K′ Kc would give its c a different semantic than in Bc.
They were an odd mix between a casefolding of the papers and remnants of the FORTRAN, leading to such things as
bc
for “𝐵 complete” next toBc
for 𝐵𝑐, j for 𝐽 where the paper calls an index 𝑗, etc.Also align the Biber/XeLaTeX bibliography with the one in the header file.