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

Additional orbit properties #717

Merged
merged 2 commits into from
Jul 2, 2019
Merged

Conversation

Sedictious
Copy link
Member

@Sedictious Sedictious commented Jul 1, 2019

Adds 3 orbit properties:

  • Angular momentum

  • Mean anomaly

  • Time elapsed from last perifocal passage

  • Tests taken from Curtis, H. D. (2005). Orbital mechanics for engineering students, p115, Example 3.1

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #717 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #717      +/-   ##
==========================================
+ Coverage   86.09%   86.14%   +0.04%     
==========================================
  Files          52       52              
  Lines        2640     2648       +8     
  Branches      221      221              
==========================================
+ Hits         2273     2281       +8     
  Misses        310      310              
  Partials       57       57
Impacted Files Coverage Δ
src/poliastro/twobody/orbit.py 91.4% <100%> (+0.18%) ⬆️

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 09c469b...9d40fdc. Read the comment docs.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments!

orbit = Orbit.from_classical(attractor, a, ecc, _a, _a, _a, nu)
orbit_M = orbit.M

assert_allclose(orbit_M.value, expected_mean_anomaly.value, rtol=1e-2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an assert_quantity_allclose in Astropy that you can use https://astropy.readthedocs.io/en/stable/api/astropy.tests.helper.assert_quantity_allclose.html

orbit = Orbit.from_classical(attractor, a, ecc, _a, _a, _a, nu)
orbit_h_mom = orbit.h_mom

assert_allclose(orbit_h_mom.value, expected_ang_mom.value, rtol=1e-2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

nu = 120 * u.deg

orbit = Orbit.from_classical(attractor, a, ecc, _a, _a, _a, nu)
orbit_h_mom = orbit.h_mom
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of h_mom, what about h_mag (of "magnitude")?

orbit = Orbit.from_classical(attractor, a, ecc, _a, _a, _a, nu)
orbit_t_p = orbit.t_p

assert_allclose(orbit_t_p.value, expected_t_p.value, rtol=1e-2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@property
def h_mom(self):
"""Angular momentum. """
h_mom = np.linalg.norm(self.h_vec) * u.km ** 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using np.linalg.norm, you can use poliastro.util.norm, which respects the units

@@ -216,12 +216,29 @@ def h_vec(self):
h_vec = np.cross(self.r.to(u.km).value, self.v.to(u.km / u.s)) * u.km ** 2 / u.s
return h_vec

@property
def h_mom(self):
"""Angular momentum. """
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that this is the specific angular momentum. In mechanics, they call "specific" to the magnitudes without mass, like this case, see https://www.wikiwand.com/en/Specific_relative_angular_momentum

src/poliastro/twobody/orbit.py Show resolved Hide resolved
@astrojuanlu astrojuanlu merged commit 05572a5 into poliastro:master Jul 2, 2019
@astrojuanlu
Copy link
Member

Merging, thanks!

@Sedictious Sedictious deleted the czml branch July 28, 2019 17:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants