-
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
Change the orbital analyser to use the Trajectory interface #3519
Merged
Merged
Changes from 1 commit
Commits
Show all changes
65 commits
Select commit
Hold shift + click to select a range
5d2e8cd
Start to use Trajectory, but unwinding makes our life harder.
pleroy 600bbf0
A promising attempt
eggrobin af60f79
the integrator is confused
eggrobin f0bfe5f
merge
eggrobin 95a1d7b
unwinding
eggrobin c95b242
unwinding that works
eggrobin 437f173
more traces
eggrobin 788ca62
back to fixed step
eggrobin 0e66b9a
bad unwinding
eggrobin 8400e7e
An attempt at Clenshaw-Curtis that does not work too badly
eggrobin c3501d4
do we really need dense output?
eggrobin 5414a4d
Merge remote-tracking branch 'la-vache/master' into better-integration
eggrobin ca81bbd
use RK4
eggrobin d19fbd3
a bug
eggrobin 0d6dbd6
Trying to throw more automated Clenshaw Curtis at the problem
eggrobin b9ca286
do interpolation correctly
eggrobin 8ac094d
compute osculating elements, and notice the noise
eggrobin 50959b2
Merge branch 'better-integration' of https://github.com/eggrobin/Prin…
pleroy 5e8ed38
ELMS.
pleroy e5d28e6
Merge.
pleroy 2de7b6d
Merge branch 'Orbit' into Benchmarque
pleroy 9bc51ed
AB order 6.
pleroy 44bffc8
Merge branch 'Benchmark' into Benchmarque
pleroy afcab79
remove logger and don’t densely sample outside of tests
eggrobin 024d755
Avoid allocations in the integrator.
pleroy ad1eebd
Avoid allocation in the ODE's equation.
pleroy b3ffd31
Unused code.
pleroy c9c0dcd
Move instead of copy.
pleroy 4f40081
Merge.
pleroy 5ca5aa3
Adjust for non-vector ODEs.
pleroy 6286a1f
Je n'en sais rien.
pleroy aaad34a
Let's make a pause.
pleroy 536f97b
A benchmark with a more inclined trajectory.
pleroy 38970e6
Cleaned up the Augean stables.
pleroy c100c60
It kind-of works.
pleroy 0a31db0
All tests passing.
pleroy 58440f8
Cleanup.
pleroy d866566
Bring the projects back alive.
pleroy 6fc31b8
Fix a compilation error.
pleroy 2342674
Minor fixes.
pleroy 04b2c47
Tolerances.
pleroy 8f3720c
Lint.
pleroy 1c290e3
Parameterization of Clenshaw-Curtis.
pleroy 47a87f5
Specific downsampling tolerance for the orbit analyser.
pleroy bbe0517
Smaller tolerance.
pleroy 413773c
Fix compilation error.
pleroy 89129e7
Merge branch 'Benchmarque' of https://github.com/pleroy/Principia int…
pleroy 35612b0
Heun
eggrobin cef4f0b
remove bad condition
eggrobin 5629624
Compile out the logging.
pleroy 86167ae
Mathematica logging.
pleroy ec5d1c5
Moal traces.
pleroy df94c90
Merge.
pleroy 5127615
Code cleanup and better tolerances.
pleroy 778f2eb
Bring back the tests.
pleroy a547281
Fix the tolerances.
pleroy 9b6c6dd
Fixed-step integrator in test.
pleroy 778d1a8
Minor cleanup.
pleroy c10ba43
More funky tolerance.
pleroy 836ad9e
Logging the errors.
pleroy 34d3ebc
Combined.
pleroy 98cbeb8
Remove bad tests.
pleroy 6302dc3
Benchmark downsampling tolerance.
pleroy b30a10b
After egg+phl's review.
pleroy 3d4be26
Compilation error.
pleroy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Notes from various experiments on this test:
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.
With a tolerance of 10 m, the trajectory has 149 776 points.
With a tolerance of 1 m, the trajectory has 259 023 points, but the semimajor axis range is only 14 909.5 km .. 14 910.7 km. So we should just lower the tolerance for the orbit analysers.
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.
Further investigation: at 10 cm the range becomes 14 909.9 km .. 14 910.3 km, which seems like a relevant difference given the size of the range under consideration. Lowering the tolerance further does not change the range to a precision of 100 m; in particular that range is the same in the absence of downsampling.
There is a separate issue, which is that the estimated range is highly sensitive to the value of 24 points per period (with no downsampling, 96 points gives a range of 14 909.7 km .. 14 911.0 km, and 120 points a range of 14 910.5 km .. 14 911.0 km. This is because, contrary to the apsides, we make no attempt to interpolate when estimating the extrema.
This is a problem, but it is a problem we had already (we got our arbitrary samples from the integration of motion followed by downsampling instead of getting them from the averaging of elements, but there was no attempt at interpolation either). We should now have the tools to tackle it properly, since we are already doing most of the work needed to compute the derivatives of the mean elements; but this is a matter for another pull request.
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.
Hm, no, this does not make sense. We would not be getting maxima larger than the correct one nor minima smaller than the correct one just because of poor sampling. Is the integrator not converged enough?
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.
Indeed; the integrators converge only to order 1 or 2 (at least at the precisions that are relevant here), because the of the discontinuities in the higher derivatives of the downsampled trajectory.
Aside: it is clear that here we want high enough precision that a dozen points per period is not enough, and for an elliptic orbit that means we benefit from a variable stepsize.
Possibilities: Heun–Euler (but that is unsatisfactory, it is mostly cutting our losses), or lowering the downsampling tolerance (maybe to 1 mm?) until Dormand–Prince converges properly.