Use polynomials to implement the continuous trajectories #1704
Conversation
|
retest this please |
|
This breaks |
|
Ah nevermind, this is not a test that actually uses Prolong, so it is polynomial evaluation + Cartesian to Keplerian conversion; the error of the former went up as expected. |
| @@ -323,6 +323,7 @@ Derivative<Value, Argument> | |||
| HornerEvaluator<Value, Argument, degree>::EvaluateDerivative( | |||
| Coefficients const& coefficients, | |||
| Argument const& argument) { | |||
| // Starting at 1 prevents us from having polynomials of degree 0. | |||
There was a problem hiding this comment.
Should we special-case this and return 0?
There was a problem hiding this comment.
We could, although it's hard to be excited about degree 0. Maybe in a future PR...
| // never need to extract their |t_min|. Logically, the |t_min| for a | ||
| // polynomial is the |t_max| of the previous one. The first polynomial has a | ||
| // |t_min| which is |*first_time_|. | ||
| struct InstantPolynomialPair { |
There was a problem hiding this comment.
So we end up having that pair after all... Is it still worth having the reference instant replicated in the polynomial itself, or would it make sense to use polynomials of Time?
There was a problem hiding this comment.
Well, really we need a triple. Previously it was {t_min, t_max, series} now it's effectively {t_mid, t_max, polynomial}. I prefer the new code because there is less drudgery in ContinuousTrajectory.
| struct InstantPolynomialPair { | ||
| InstantPolynomialPair( | ||
| Instant t_max, | ||
| not_null<std::unique_ptr<Polynomial<Displacement<Frame>, Instant>>> |
There was a problem hiding this comment.
Why are they not Position-valued?
There was a problem hiding this comment.
Mostly because they were not in Chebyshev, and I wanted to be able to compare apples to apples. We could look into this in the future (it would probably be less clunky than with Chebyshev because the metaprogramming would just do the right thing). Nod to #770.
physics/ephemeris_test.cpp
Outdated
| @@ -499,7 +499,7 @@ TEST_P(EphemerisTest, EarthProbe) { | |||
| // The solution is a line, so the rounding errors dominate. Different | |||
| // compilers result in different errors and thus different numbers of steps. | |||
There was a problem hiding this comment.
Change this to "different libms" as we now know the compilers are innocent (unless we get the flags wrong). The above involves std::pow to adapt the step size. I am unsure why earth_positions has variable errors, but there must be something similar somewhere...
The benchmarks are uniformly faster. Some (the ones that only exercise
Prolong) marginally so (4-5%), probably because the Newhall construction got a wee bit faster. Others (the ones that exerciseEvaluatePosition) by a factor that goes up to 1.4x-1.5x. The numbers are as follows:Before:
After: