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

New powerful twobody propagators #718

Merged
merged 13 commits into from Jul 9, 2019
Merged

New powerful twobody propagators #718

merged 13 commits into from Jul 9, 2019

Conversation

@jorgepiloto
Copy link
Member

jorgepiloto commented Jul 1, 2019

The following PR tries to solve for #475 and #714. It includes the following new propagators for solving the Kepler equation:

New features and bug solutions:

  1. These algorithms work really well and do not require evaluation of transcendental functions either several iterations to converge to an accurate solution.

  2. It was found that orbit from #474, related with #475, is nearly equatorial and therefore singularities appear and produce the error.

  3. The test marked with the skipped command is now super fast if solved with some of the new propagators and not making so many iterations with the mean_motion.

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:kepler branch from cd619e8 to 63fc647 Jul 2, 2019
@jorgepiloto jorgepiloto changed the title Kepler improved propagator New powerful propagators Jul 2, 2019
@jorgepiloto jorgepiloto changed the title New powerful propagators New powerful twobody propagators Jul 2, 2019
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 2, 2019

There is a bug with the markley propagator:

(Pdb) p ss0
15 x 8088 km x 180.0 deg (GCRS) orbit around Earth (♁) at epoch J2000.000 (TT)
(Pdb) p ss0.r.to(u.km)
<Quantity [8000., 1000.,    0.] km>
(Pdb) p ss0.propagate(0 * u.s, method=cowell).r.to(u.km)
<Quantity [8000., 1000.,    0.] km>
(Pdb) p ss0.propagate(1 * u.s, method=cowell).r.to(u.km)
<Quantity [7999.49695737,  999.49961973,    0.        ] km>
(Pdb) p ss0.propagate(0 * u.s, method=kepler_improved).r.to(u.km)
<Quantity [ 8.06225775e+03, -1.79061317e-11,  2.19235174e-27] km>
(Pdb) p ss0.propagate(1 * u.s, method=kepler_improved).r.to(u.km)
<Quantity [ 8.06169653e+03, -4.34121516e-01,  5.31645525e-17] km>
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 2, 2019

We checked and it's because the orbit is equatorial. With non-equatorial orbits it works like a charm!

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 2, 2019

Possibly relevant: #383

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 2, 2019

By the way:

So ~ many ~ propagators 😍

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jul 4, 2019

Codecov Report

Merging #718 into master will increase coverage by 0.81%.
The diff coverage is 93.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #718      +/-   ##
==========================================
+ Coverage   86.14%   86.95%   +0.81%     
==========================================
  Files          52       52              
  Lines        2648     2928     +280     
  Branches      221      249      +28     
==========================================
+ Hits         2281     2546     +265     
- Misses        310      317       +7     
- Partials       57       65       +8
Impacted Files Coverage Δ
src/poliastro/twobody/propagation.py 92.07% <86.66%> (-4.35%) ⬇️
src/poliastro/core/propagation.py 94.71% <95.33%> (+6.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05572a5...8f0dc8d. Read the comment docs.

Copy link
Member

Juanlu001 left a comment

I gave a quick, first pass. Excellent work!

src/poliastro/tests/tests_twobody/test_propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved
src/poliastro/twobody/propagation.py Outdated Show resolved Hide resolved

# Solve first for eccentricity and mean anomaly
p, ecc, inc, raan, argp, nu = rv2coe(k, r0, v0)
if ecc >= 1.0:

This comment has been minimized.

Copy link
@Juanlu001

Juanlu001 Jul 4, 2019

Member

Does it work for the case ecc == 1.0?

s = ecc * np.sin(M)
psi = s / np.sqrt(1 - 2 * c + ecc ** 2)
f = 1.0
while f ** 2 >= rtol and n <= numiter:

This comment has been minimized.

Copy link
@Juanlu001

Juanlu001 Jul 4, 2019

Member

What happens if the maximum number of iterations is reached?

@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 8, 2019

Only failure:

docstring of poliastro.twobody.propagation.pimienta:24:Unexpected indentation.

This is almost good to go!

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:kepler branch from 76b4acf to 9f8b0e6 Jul 8, 2019
@jorgepiloto jorgepiloto force-pushed the jorgepiloto:kepler branch from 9f8b0e6 to 8f0dc8d Jul 8, 2019
@jorgepiloto

This comment has been minimized.

Copy link
Member Author

jorgepiloto commented Jul 8, 2019

I spent the whole afternoon trying to fix some of those NotImplementedError but could not solve them. Since this pull request is getting bigger (sorry for that and thanks for the reviews) I think is enough for the moment.

More information on propagators current status is available at: #714 (comment)

@Juanlu001 Juanlu001 merged commit 4a95aab into poliastro:master Jul 9, 2019
10 checks passed
10 checks passed
ci/circleci: coverage Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: quality Your tests passed on CircleCI!
Details
ci/circleci: test_py35 Your tests passed on CircleCI!
Details
ci/circleci: test_py36 Your tests passed on CircleCI!
Details
ci/circleci: test_py37 Your tests passed on CircleCI!
Details
codeclimate Approved by Juan Luis Cano Rodríguez.
Details
codecov/patch 93.95% of diff hit (target 86.14%)
Details
codecov/project 86.95% (+0.81%) compared to 05572a5
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 9, 2019

Awesome work @jorgepiloto, this is in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.