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

Orekit validation first approach #4

Merged
merged 8 commits into from
Dec 20, 2020
Merged

Conversation

jorgepiloto
Copy link
Member

@jorgepiloto jorgepiloto commented Dec 7, 2020

This pull request is a first approach to poliastro's validation path. It makes use of the Orekit python wrapper and implements the following features:

  • A class under the name OrekitOrbit, who's usage is similar to poliastro's Orbit class. Both of them try to model Keplerian orbits, which can be defined from vectors or classic orbital elements.

  • A simple test regarding the conversion between RV and COE.

⚠️ Orekit does not provide functions like rv2coe or coe2rv, since conversion is done directly in each of the available constructors of the KeplerianOrbit class in the Orekit library ⚠️

Also notice the addition of the orekit-data.zip file, which holds necessary data for Orekit to properly work: ephemeris, time files... We might discuss if this data should be included within this repo or final user is required to download it from official channels.

@astrojuanlu astrojuanlu self-requested a review December 8, 2020 00:37
@astrojuanlu
Copy link
Member

We might discuss if this data should be included within this repo or final user is required to download it from official channels.

Let's not put it in the repo 🙏 Otherwise it will make the whole git history too sluggish. If it's a common Orekit thing we can point to the relevant documentation, or even provide a script to download it.

I'll take a look at this tomorrow!

@jorgepiloto
Copy link
Member Author

I just saw that the Orekit wrapper provides a download data function.

On the other hand, if the setup_orekit_dir() does not find the orekit-data.zip file in the local directory, en error message is shown but no exception is raised. This is weird when trying to manage program workflow...

@jorgepiloto
Copy link
Member Author

Some updates on this PR regarding previous suggestions and feedback

I came up with the following structure:

orekit/
├── README.md
├── requirements.txt
├── src/
└── tests/

The src/ directory holds all logical files while the tests/ one is devoted to validation unit tests. In addition, a pytest.ini and conftest.py files are placed under this last folder, so anyone can run the tests following the same configuration.

Regarding orekit-data.zip:

Let's not put it in the repo pray Otherwise it will make the whole git history too sluggish. If it's a common Orekit thing we can point to the relevant documentation, or even provide a script to download it.

Now, there exists a setup_orekit_env() function under src/utils.py which checks if data is available in local machine. If not, it automatically downloads it from official sources and points orekit internals to data location, which I selected to be within src/data/.

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.

This is on the right path! Left a few comments.

README.md Outdated Show resolved Hide resolved
orekit/README.md Outdated Show resolved Hide resolved
orekit/README.md Outdated Show resolved Hide resolved
orekit/README.md Outdated Show resolved Hide resolved
orekit/src/utils.py Outdated Show resolved Hide resolved
orekit/tests/test_twobody_orbit.py Show resolved Hide resolved
@jorgepiloto
Copy link
Member Author

Sorry, I just forgot to update the README.md installation section guide. I will solve for this this night.

@jorgepiloto
Copy link
Member Author

I just added a propagate() method and its corresponding validation test, which passes without any problem. I think with all these features, we are ready to start building more complex tests such us maneuvers validation.

@jorgepiloto jorgepiloto marked this pull request as ready for review December 20, 2020 23:39
orekit/src/utils.py Outdated Show resolved Hide resolved
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.

This is good to go! 🚀

@astrojuanlu astrojuanlu merged commit 291174c into poliastro:master Dec 20, 2020
@astrojuanlu
Copy link
Member

Merging! Thanks @jorgepiloto for taking this up and the patience 💪🏽

@jorgepiloto jorgepiloto deleted the orekit branch December 22, 2020 12:50
@astrojuanlu
Copy link
Member

Oh, my bad... I now see that we merged orekit-data.zip here.

astrojuanlu added a commit that referenced this pull request Dec 26, 2020
Orekit validation first approach
@astrojuanlu
Copy link
Member

⚠️ HISTORY OF THIS PULL REQUEST HAS BEEN REWRITTEN ⚠️

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

Successfully merging this pull request may close these issues.

None yet

2 participants