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

RKNG #1923

Merged
merged 23 commits into from
Sep 4, 2018
Merged

RKNG #1923

merged 23 commits into from
Sep 4, 2018

Conversation

eggrobin
Copy link
Member

@eggrobin eggrobin commented Sep 2, 2018

First cut, more tests to come (probably for the RKN too, the existing tests don't check the order).

Fix a bug in EstrinEvaluator (could not evaluate constants).

It may make sense to factor the implementations of the RKN and RKNG method with some template magic to ignore the a′ in the former.

@pleroy pleroy added the LGTM label Sep 2, 2018

// This class solves ordinary differential equations of the form
// q″ = f(q, q′, t)
// using an embedded generalized Runge-Kutta-Nyström (RKNG) method. We follow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not called GRKN?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¯\_(ツ)_/¯

Fine (1987) says “Runge-Kutta-Nyström Generalized (RKNG)”.
Sharp and Fine (1992) say “generalized Runge-Kutta-Nyström (RKNG)”.
Dormand (1996) says “The designation ‘RKNG’ is used because the shorter ‘RKN’ abbreviation is usually reserved for formulae applicable to the special equation (14.2)”, and does not expand the acronym.
Murua (1998) says “RKNG” without expanding the acronym.


// Alternative notations use a and b for the velocity Runge Kutta matrix and
// weights: Murua (1998), Runge-Kutta-Nyström methods for general second Order
// ODEs with application to multi-body Systems, uses (c, α, a, β̂, b̂, β, b),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a hat above a comma here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your font is bad and you should feel bad.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or more likely, GitHub's font is bad and it should feel bad.

template<typename Method, typename Position>
EmbeddedExplicitGeneralizedRungeKuttaNyströmIntegrator<Method, Position>::
EmbeddedExplicitGeneralizedRungeKuttaNyströmIntegrator() {
// the first node is always 0 in an explicit method.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (also in the RKN).


// The first stage of the Runge-Kutta-Nyström iteration. In the FSAL case,
// |first_stage == 1| after the first step, since the first RHS evaluation has
// already occured in the previous step. In the non-FSAL case and in the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

occurred

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (also for the RKNs).


class EmbeddedExplicitGeneralizedRungeKuttaNyströmIntegratorTest
: public ::testing::Test {};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// b̂ for the position weights of the high-order method;
// b̂′ for the velocity weights of the high-order method;
// b for the position weights of the low-order method;
// b′ for the velocity weights of the low-order method.
// See Dormand, El-Mikkawy and Prince (1986),
// Families of Runge-Kutta-Nyström formulae, for an example.
// Note that other notations exist for the weights:
// المكاوى and رحمو‎ (2003), A new optimized non-FSAL embedded
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we use the latinized form from the papers?

@@ -16,9 +16,9 @@ constexpr int FloorOfPowerOf2(int const n) {
return n == 0 ? 0 : n == 1 ? 1 : FloorOfPowerOf2(n >> 1) << 1;
}

// Ceiling log2 of n. 8 -> 3, 7 -> 2.
// Ceiling log2 of n, or 0 for n = 0. 8 -> 3, 7 -> 2.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that Estrin evaluation compiles for degree 0.

@pleroy
Copy link
Member

pleroy commented Sep 3, 2018

retest this please

@pleroy pleroy merged commit 31a221a into mockingbirdnest:master Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants