-
-
Notifications
You must be signed in to change notification settings - Fork 238
Add LVLH frame and geoid intersection method #839
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
Open
jamesgrimmett
wants to merge
14
commits into
skyfielders:master
Choose a base branch
from
jamesgrimmett:add-geoid-intersection-methods-and-lvlh-frame
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
38971e4
add vector intersection methods
jamesgrimmett d0b59d8
add lvlh inertial frame
jamesgrimmett fcf15ae
add geoid intersection example to docs
jamesgrimmett 96316b3
fix typo in example docs
jamesgrimmett eef729f
add elevation check to intersection tests
jamesgrimmett 836a366
add a custom __repr__ method for inertial frame objects
jamesgrimmett d1465e9
refactor attitude from framelib to sgp4lib
jamesgrimmett 21adbf8
simplify calculation of lvlh frame rotation
jamesgrimmett 2c3adf9
reduce calculations in attitude calculation and represent result usin…
jamesgrimmett 4bdc3bc
add docstring and use explicit variable names
jamesgrimmett 5bd963d
update unit tests and documentation
jamesgrimmett 97d29e6
revert unnecessary changes to framelib.IntertialFrame
jamesgrimmett c76de23
add leading underscore to intersection_of
jamesgrimmett 4188a22
avoid using deprecated Geometric class and improve docstring
jamesgrimmett File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of or in addition to this method of returning the rotation matrix itself, could it return a scipy Rotation object? it would then be easily convertible back to a rotation matrix or to other useful forms like Euler angles or quaternions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reaching out, the test data that you mentioned would be excellent! I had been meaning to spend some time thinking about additional testing strategies, so any data that you could provide would be very much appreciated.
Are you able to attach the data here? Otherwise, could you email it through to - james dot grimmett at protonmail dot com?
I think you make a good point about scipy Rotation objects being a nice, versatile way to carry the rotation information. I would lean toward keeping the return value as a matrix, however. Mainly to remain consistent with several other
rotation_atfunctions scattered throughout theskyfield.framelibmodule, and also to avoid adding ascipydependency. That said, I would be curious what @brandon-rhodes opinion is - it may be an option that has been considered before?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesgrimmett — I agree entirely. SciPy is even larger than NumPy, and it would be a shame to nearly double the install size of Skyfield just for a single feature by adding it as a dependency. The
Rotationobject also strikes me as a bit expensive, in terms of its setup cost (as I'm reading through itsfrom_matrix()builder method), for a basic operation in Skyfield.If there's a good use case for turning Skyfield rotations into
Rotationobjects, let's just plan on showing in the documentation how it would work, so that folks who need SciPy can import it into their own code and build theRotation.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that there's no need to add a large dependency for this. But want to chime in that scipy is very close to adding a representation of
RigidTransform's that will allow for compactly representing coordinate frames (Rotations plus translations). May also be useful for things: scipy/scipy#22267Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my PhD I hacked out some experiments with the integration of Skyfield and LVLH. I was never able to validate the results or discuss them in detail with anyone, but they might be helpful, or at least the Stack OverFlow articles I found :) So no guarantee on this code: https://github.com/Ly0n/in-orbit-validation-simulator/blob/main/moon_venus_sentinel_5P_thesis.ipynb
There is now also a new numpy integration for quaternions that might be interesting: https://github.com/moble/quaternion