This repository has been archived by the owner on Oct 14, 2023. It is now read-only.
Refactor math (was: poliastro.ephem cleanup)
#1434
Merged
Conversation
This file contains 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
|
Pro tip:
|
astrojuanlu
force-pushed
the
ephem-cleanup
branch
from
afe5ae7 to
013dd12
Compare
|
Next in line: astrojuanlu#13 |
|
Hmmm slow tests got really slow? 🤔 |
Astropy 5.0 supports Moon velocity!
astrojuanlu
force-pushed
the
ephem-cleanup
branch
from
013dd12 to
ce215d8
Compare
|
|
astrojuanlu
commented
Jan 9, 2022
src/poliastro/ephem.py
Outdated
|
|
||
| r_values[i] = r.xyz.to(u.km) | ||
| def interpolant(t): |
There was a problem hiding this comment.
This probably poses a big performance regression and can't be merged as it stands now 😭
|
I have some ideas on how I could tackle this but I'll need more time. For now, I'll partially revert this and merge. |
astrojuanlu
force-pushed
the
ephem-cleanup
branch
from
ce215d8 to
46b0f87
Compare
astrojuanlu
changed the title
poliastro.ephem cleanuppoliastro.ephem cleanup)
This reverts commit 7770505.
astrojuanlu
force-pushed
the
ephem-cleanup
branch
from
46b0f87 to
307f6a2
Compare
|
Looking much better now. |
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Notable changes:
RewriteHad to revert this because it caused a massive slowdown, see below.build_ephem_interpolantin terms of existing classes. I tried to remove its usage but it will require a bit more care.poliastro._mathpackage with all mathematical functions not strictly related to Astrodynamics, so we can easily identify (and possibly refactor) dependencies with SciPy. I decided to make it private but maybe we shouldn't be so overprotective, I'm open to suggestions... 😋