Skip to content
This repository has been archived by the owner on Oct 14, 2023. It is now read-only.

Propagating 0 seconds with cowell fails #328

Closed
shreyasbapat opened this issue Feb 27, 2018 · 20 comments
Closed

Propagating 0 seconds with cowell fails #328

shreyasbapat opened this issue Feb 27, 2018 · 20 comments
Assignees
Milestone

Comments

@shreyasbapat
Copy link
Member

shreyasbapat commented Feb 27, 2018

Currently we can only use the basic mean_method function to propagate orbits. As the Orbit.Sample() can now accept other propagators as well, we must be able to use cowell method as well.
馃悶 Problem
We can't use the cowell's method and other propagators to propagate the orbit because of differences in the parameters passed in the normal propagator while using Orbit.Sample().

馃枼 Please paste the output of following commands

conda info -a

python -c "import poliastro.testing; poliastro.testing.test()"
python3 -c "import poliastro.testing; poliastro.testing.test()"
============================= test session starts ==============================
platform linux -- Python 3.6.3, pytest-3.4.1, py-1.5.2, pluggy-0.6.0
rootdir: /home/shreyas/poliastro, inifile: setup.cfg
plugins: cov-2.5.1
collected 141 items

poliastro/src/poliastro/tests/test_bodies.py ...... [ 4%]
poliastro/src/poliastro/tests/test_coordinates.py ... [ 6%]
poliastro/src/poliastro/tests/test_hyper.py ........... [ 14%]
poliastro/src/poliastro/tests/test_iod.py ......... [ 20%]
poliastro/src/poliastro/tests/test_jit.py ... [ 22%]
poliastro/src/poliastro/tests/test_maneuver.py ...... [ 26%]
poliastro/src/poliastro/tests/test_patched_conics.py .. [ 28%]
poliastro/src/poliastro/tests/test_plotting.py ......... [ 34%]
poliastro/src/poliastro/tests/test_plotting3d.py ...... [ 39%]
poliastro/src/poliastro/tests/test_stumpff.py ... [ 41%]
poliastro/src/poliastro/tests/test_twobody.py .. [ 42%]
poliastro/src/poliastro/tests/test_util.py ....... [ 47%]
poliastro/src/poliastro/tests/tests_neos/test_dastcom5.py ......... [ 53%]
poliastro/src/poliastro/tests/tests_neos/test_neos_neows.py ...... [ 58%]
poliastro/src/poliastro/tests/tests_twobody/test_angles.py ....... [ 63%]
poliastro/src/poliastro/tests/tests_twobody/test_decorators.py .. [ 64%]
poliastro/src/poliastro/tests/tests_twobody/test_orbit.py .............. [ 74%]
..... [ 78%]
poliastro/src/poliastro/tests/tests_twobody/test_propagation.py ........ [ 83%]
. [ 84%]
poliastro/src/poliastro/tests/tests_twobody/test_sample.py ............ [ 92%]
poliastro/src/poliastro/tests/tests_twobody/test_states.py .......... [100%]

========================= 141 passed in 21.47 seconds ==========================

馃幆 Goal
To be able to Propagate the orbit using different propagators.

馃挕 Possible solutions

馃搵 Steps to solve the problem

  • Comment below about what you've started working on.
  • Add, commit, push your changes
  • Submit a pull request and add this in comments - Addresses #<put issue number here>
  • Ask for a review in comments section of pull request
  • Celebrate your contribution to this project 馃帀
@shreyasbapat shreyasbapat changed the title Changing the other propagators so they can be used for propagating the orbit through Orbit.Sample() Using other propagation methods for propagating the orbit through Orbit.Sample() Feb 27, 2018
@astrojuanlu
Copy link
Member

Can you check if this is still the case after merging #322? Also, is it a specific problem of cowell or affects other propagators as well? Can you ellaborate?

@shreyasbapat
Copy link
Member Author

shreyasbapat commented Feb 28, 2018

Yes! It's still the case. Wait the names have changed and hence it works for mean_motion and kepler .But not for cowell.

---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-16-c620f81ccde0> in <module>()
----> 1 iss.sample(None,method=cowell)

~/poliastro/src/poliastro/twobody/orbit.py in sample(self, values, method)
    292         """
    293         if values is None:
--> 294             return self.sample(100, method)
    295 
    296         elif isinstance(values, int):

~/poliastro/src/poliastro/twobody/orbit.py in sample(self, values, method)
    308                 nu_values = np.insert(nu_values, 0, self.ecc)
    309 
--> 310             return self.sample(nu_values, method)
    311 
    312         elif hasattr(values, "unit") and values.unit in ('rad', 'deg'):

~/poliastro/src/poliastro/twobody/orbit.py in sample(self, values, method)
    312         elif hasattr(values, "unit") and values.unit in ('rad', 'deg'):
    313             values = self._generate_time_values(values)
--> 314         return (values, self._sample(values, method))
    315 
    316     def _sample(self, time_values, method=mean_motion):

~/poliastro/src/poliastro/twobody/orbit.py in _sample(self, time_values, method)
    317         values = np.zeros((len(time_values), 3)) * self.r.unit
    318         for ii, epoch in enumerate(time_values):
--> 319             rr = self.propagate(epoch, method).r
    320             values[ii] = rr
    321 

~/poliastro/src/poliastro/twobody/orbit.py in propagate(self, epoch_or_duration, method, rtol)
    254             time_of_flight = time.TimeDelta(epoch_or_duration)
    255 
--> 256         return propagate(self, time_of_flight, method=method, rtol=rtol)
    257 
    258     def sample(self, values=None, method=mean_motion):

~/poliastro/src/poliastro/twobody/propagation.py in propagate(orbit, time_of_flight, method, rtol, **kwargs)
    213                   time_of_flight.to(u.s).value,
    214                   rtol=rtol,
--> 215                   **kwargs)
    216     return orbit.from_vectors(orbit.attractor, r * u.km, v * u.km / u.s, orbit.epoch + time_of_flight)
    217 

~/poliastro/src/poliastro/twobody/propagation.py in cowell(k, r0, v0, tof, rtol, ad, callback, nsteps)
    102         r, v = rr.y[:3], rr.y[3:]
    103     else:
--> 104         raise RuntimeError("Integration failed")
    105 
    106     return r, v

RuntimeError: Integration failed

@shreyasbapat
Copy link
Member Author

I realized that I could forget this hence I put it here.

@astrojuanlu
Copy link
Member

This looks like a convergence problem with the cowell method, rather than an API problem:

In [25]: iss.propagate(0 * u.s, method=cowell)
/home/juanlu/.miniconda36/envs/poliastro36/lib/python3.6/site-packages/scipy/integrate/_ode.py:1095: UserWarning: dop853: step size becomes too small
  self.messages.get(istate, unexpected_istate_msg)))
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-25-5ec3bbe8e578> in <module>()
----> 1 iss.propagate(0 * u.s, method=cowell)

~/Development/poliastro/poliastro-library/src/poliastro/twobody/orbit.py in propagate(self, epoch_or_duration, method, rtol)
    254             time_of_flight = time.TimeDelta(epoch_or_duration)
    255 
--> 256         return propagate(self, time_of_flight, method=method, rtol=rtol)
    257 
    258     def sample(self, values=None, method=mean_motion):

~/Development/poliastro/poliastro-library/src/poliastro/twobody/propagation.py in propagate(orbit, time_of_flight, method, rtol, **kwargs)
    213                   time_of_flight.to(u.s).value,
    214                   rtol=rtol,
--> 215                   **kwargs)
    216     return orbit.from_vectors(orbit.attractor, r * u.km, v * u.km / u.s, orbit.epoch + time_of_flight)
    217 

~/Development/poliastro/poliastro-library/src/poliastro/twobody/propagation.py in cowell(k, r0, v0, tof, rtol, ad, callback, nsteps)
    102         r, v = rr.y[:3], rr.y[3:]
    103     else:
--> 104         raise RuntimeError("Integration failed")
    105 
    106     return r, v

RuntimeError: Integration failed

of course, sending a value of 0 seconds should return just the same position :) An easy issue to fix!

@astrojuanlu astrojuanlu changed the title Using other propagation methods for propagating the orbit through Orbit.Sample() Propagating 0 seconds with cowell fails Feb 28, 2018
@astrojuanlu astrojuanlu added the good first issue Easy tasks for beginners label Feb 28, 2018
@shreyasbapat
Copy link
Member Author

Yep! True!

@astrojuanlu astrojuanlu added this to the 0.9 milestone Mar 4, 2018
@astrojuanlu
Copy link
Member

I think that iss.sample(method=cowell) won't raise the error after this was merged:

#330

However, iss.propagate(0 * u.s, method=cowell) or iss.sample([0] * u.s, method=cowell) should do.

@shreyasbapat
Copy link
Member Author

Working on it!

@astrojuanlu
Copy link
Member

@shreyasbapat C'mon, leave some easy issues for the newcomers 馃槣

@shreyasbapat
Copy link
Member Author

@Juanlu001 Not Working Now 馃槈

@astrojuanlu astrojuanlu removed this from the 0.9 milestone Apr 18, 2018
@astrojuanlu
Copy link
Member

I still want some newcomer to fix this issue, so I'm removing the milestone.

@astrojuanlu
Copy link
Member

Is this still the case with the new integrator @nikita-astronaut?

@nikita-astronaut
Copy link
Contributor

This still fails, but the error is different: the new integrator does not do anything and returns nothing :) I will fix this in the next PR

@nikita-astronaut
Copy link
Contributor

Hey, @Juanlu001

After the last PR, 'iss.propagate(0 * u.s, method=cowell)' forks nice. But

>>> iss.sample(np.array([0]) * u.s, method=cowell) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/astronaut/anaconda3/lib/python3.6/site-packages/poliastro/twobody/orbit.py", line 334, in sample return values, self._sample(values, method) File "/home/astronaut/anaconda3/lib/python3.6/site-packages/poliastro/twobody/orbit.py", line 339, in _sample values, _ = cowell(self, (time_values - self.epoch).to(u.s).value) File "/home/astronaut/anaconda3/lib/python3.6/site-packages/astropy/time/core.py", line 1411, in __rsub__ return -out TypeError: bad operand type for unary -: 'Time'

@nikita-astronaut
Copy link
Contributor

Seems like providing of time list from outside of orbit.sample works incorrect, but we don't use it so tests do not fall.

The reason is that now one needs to explicitly cast u.s to Time instance. Am I right?

@astrojuanlu
Copy link
Member

Seems like providing of time list from outside of orbit.sample works incorrect, but we don't use it so tests do not fall.

Then we need more tests!

The reason is that now one needs to explicitly cast u.s to Time instance. Am I right?

poliastro, not the user :)

@astrojuanlu
Copy link
Member

Let's try to fix this one before the next release.

@astrojuanlu astrojuanlu added this to the 0.10 milestone Jul 13, 2018
@astrojuanlu
Copy link
Member

Well... Or maybe the next after that!

@astrojuanlu astrojuanlu modified the milestones: 0.10, 0.11 Jul 21, 2018
@astrojuanlu astrojuanlu modified the milestones: 0.11, 0.12 Sep 7, 2018
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Sep 17, 2018
An error in sample surfaced, see poliastro#328
@astrojuanlu
Copy link
Member

I will fix this myself.

@astrojuanlu astrojuanlu self-assigned this Sep 17, 2018
@astrojuanlu astrojuanlu modified the milestones: 0.12, 0.11 Sep 17, 2018
@shreyasbapat
Copy link
Member Author

You said this was a beginner issue :/

@astrojuanlu
Copy link
Member

The original issue was, but the problem changed and we continued discussing it here:

#328 (comment)

I need to think carefully on the API and which cases do we accept, and also there's one doctest failing because of it (see #454), so I prefer to take care of it myself.

@astrojuanlu astrojuanlu removed the good first issue Easy tasks for beginners label Sep 17, 2018
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Sep 19, 2018
An error in sample surfaced, see poliastro#328
@ghost ghost removed the 1 - Ready label Sep 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants