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

WIP:adding benchmarks #16

Merged
merged 3 commits into from Jul 27, 2018

Conversation

2 participants
@nikita-astronaut
Copy link
Contributor

nikita-astronaut commented Jul 25, 2018

No description provided.

def time_propagation(method, ecc):
a = 7000 * u.km
_a = 0.0 * u.rad
if ecc < 1.0:

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 25, 2018

Member

Now that I see this, I'm not supper happy with the "discontinuity" of the Orbit.from_classical function (which we were already aware of anyway, see https://github.com/poliastro/poliastro/wiki/Parabolic-orbits:-final-decision). On the other hand, this test is backwards compatible no matter we decide regarding this particular point.

@Juanlu001
Copy link
Member

Juanlu001 left a comment

Left some comments. Regarding the setup functions, I don't care whether we use the Object-Oriented approach or the functional approach. The objects look cleaner and don't make you use module-level variables, but I don't have a strong opinion.

r = [12214.83399, 10249.46731, 0.0] * u.km
tof = 76.0 * u.min

expected_va = [2.058925, 2.915956, 0.0] * u.km / u.s

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 27, 2018

Member

The expected_* variables are not used here

expected_va = [2.058925, 2.915956, 0.0] * u.km / u.s
expected_vb = [-3.451569, 0.910301, 0.0] * u.km / u.s

va, vb = next(lambert(k, r0, r, tof))

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 27, 2018

Member

I don't think we even need variable assignment here (IDEs will complain anyway that "variables are not used" or "statement has no effect")



def time_J2_propagation_Earth():
r0 = np.array([-2384.46, 5729.01, 3050.46]) # km

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 27, 2018

Member

With this structure, if the setup takes a significant amount of time, we will get noisy results. We should move everything that is not cowell(...) to a setup function:

https://asv.readthedocs.io/en/latest/writing_benchmarks.html#setup-and-teardown-functions

orbit = Orbit.from_classical(Earth, a_ini, ecc_ini, inc_ini, raan_ini, argp_ini, nu_ini)

tof = (1.0 * u.min).to(u.s).value
r_J2, v_J2 = cowell(orbit, tof, ad=J2_perturbation,

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 27, 2018

Member

Same as above:

  • No variable assignment
  • Separate setup function
rho0 = Earth.rho0.to(u.kg / u.km**3).value # kg/km^3
H0 = Earth.H0.to(u.km).value
tof = 60 # s
r, v = cowell(orbit, tof, ad=atmospheric_drag, R=R, C_D=C_D, A=A, m=m, H0=H0, rho0=rho0)

This comment has been minimized.

@Juanlu001

Juanlu001 Jul 27, 2018

Member

Same as above

@Juanlu001 Juanlu001 merged commit ebbcf39 into poliastro:master Jul 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Jul 27, 2018

Great, thanks! I will start running them tonight.

@nikita-astronaut nikita-astronaut deleted the nikita-astronaut:adding_benchmarks_1 branch Jul 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment