-
-
Notifications
You must be signed in to change notification settings - Fork 282
Added recursive series Kepler solver for elliptical orbits #1439
Conversation
Recursive series solution solver for elliptical Kepler's problem.
Added recSeries solver to propagation __init__ file.
Performed clean up corrections to the recursive solver * removed numiter options were left in the previous commit
Corrected to simply the nu_to_M case for circular orbit (M=E=nu @ e=0)
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## main #1439 +/- ##
==========================================
- Coverage 91.93% 91.85% -0.08%
==========================================
Files 93 94 +1
Lines 4400 4444 +44
Branches 422 429 +7
==========================================
+ Hits 4045 4082 +37
- Misses 267 272 +5
- Partials 88 90 +2
Continue to review full report at Codecov.
|
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.
Amazing @JackCrusoe47 ! That's what I call "from zero to hero" 😃 Good use of GitHub, nothing to worry about there.
I have a few minor requests:
- Could you rename the module and function names to
all_lowercase
instead ofcamelCase
? to keep consistency - Could you add your method here
poliastro/src/poliastro/twobody/propagation.py
Lines 488 to 497 in 9797eb0
ELLIPTIC_PROPAGATORS = [ | |
farnocchia, | |
vallado, | |
mikkola, | |
markley, | |
pimienta, | |
gooding, | |
danby, | |
cowell, | |
] |
so it gets tested automatically?
Thank you @astrojuanlu. of course, I'll do the corrections right away. I also saw that two automatic tests are failing. Is it related to not adding my method in the tests, or would I need to modify anything further? The tests that fail are:
|
-Updated solver name to be more in line with project guidelines. -Updated zero-order solution to E = M+e (Vallado) for improved convergence
Wow! This seems to be a great addition. As the paper states,
this approach could also be a good approach to check out for the satellite collision event #1350, isn't it? |
for more information, see https://pre-commit.ci
Updated solver name to better be in line with project guidelines
Added recursive series propagator for elliptical Keplerian problem.
for more information, see https://pre-commit.ci
Correction
Corrected esle to else
for more information, see https://pre-commit.ci
Further spell errors and function names
@Yash-10 A possible method could be to use recursive series to do a fast estimation. Then once a potential collision region is identified, more accurate methods like Newton-Raphson computes the precise distance and location. P.S I apologize about the state of my commits. I still have not found a way to roll back my commits. |
fixed iteration method. tolerance for a given order varies for different M and ecc values.
Tolerance from given order computation. Very approximate, but good for low to mid tolerance values and low eccentricities order = 2 * floor( - log10( tolerance ) )
for more information, see https://pre-commit.ci
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.
5 tests have failed:
FAILED tests/tests_twobody/test_propagation.py::test_elliptic_near_parabolic[recseries-0.9]
FAILED tests/tests_twobody/test_propagation.py::test_elliptic_near_parabolic[recseries-0.99]
FAILED tests/tests_twobody/test_propagation.py::test_elliptic_near_parabolic[recseries-0.999]
FAILED tests/tests_twobody/test_propagation.py::test_elliptic_near_parabolic[recseries-0.9999]
FAILED tests/tests_twobody/test_propagation.py::test_elliptic_near_parabolic[recseries-0.99999]
which means that, with eccentricities of 0.9 and higher, the method loses a bit of accuracy.
I assume this is how the method works, so I'll assist you in relaxing the tolerance for the tests without affecting the other methods.
Unfortunately yes. The rate of convergence of the method drops to zero as the eccentricity becomes close to 1. In theory, it should work for cases up to 0.99 with a high enough order (say 20-30 for 0.99 for example). But for now the function I use to convert accuracy to order probably is underpredicting the order for 0.9 and 0.99. |
Well, I can make it work with a fixed tolerance instead of fixed recursion order. By adding three functions inside recseries.py
|
Compute eccentricity for recseries propagation Updated rtol to order function
for more information, see https://pre-commit.ci
removed extra order = order =
Added ability to choose two different methods of series recursion termination - fixed user-defined order as method='order' - fixed user-defined tolerance as method='rtol'
for more information, see https://pre-commit.ci
run recseries through user-specified tolerance
for more information, see https://pre-commit.ci
Correction: [recseries_fast(k, r0, v0, tof, method='rtol', order=8, numiter=100, rtol) for tof in tofs]
Correction recseries call
for more information, see https://pre-commit.ci
All cases more or less should work now. Now the recseries( ) solver can be called in the following two ways:
This works the same for recseries_coe( ) |
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.
Looks good to me! Will merge when the tests pass 💪🏼
Perfect. Thank you. |
A small coverage difference is not worth blocking this PR 😉 |
Congratulations @JackCrusoe47 on your first contribution to poliastro! 🎉 |
Thank you @astrojuanlu for merging my commits 🚀. I am eager to see its use by the community. 😄 |
Currently, I only included the manual series solution, where the user provides the order of recursion (instead of tolerance).
Later, I can implement a hybrid variant that uses a lookup table to select appropriate order for a mean anomaly and eccentricity and then uses the initial guess for a Newton-Raphson. Here, the user will be able to specify a tolerance, and hopefully, the table provides an order of recursion that overall reduce the cost of computing the solution. I am currently having trouble with fast lookup.
Hope I didn't mess anything up with the pull request. I have never worked with Github.
Cheers