-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Vectorize mpc.mpc_orbit
, mpc.comet_orbit
#532
base: master
Are you sure you want to change the base?
Conversation
I see that this and #526 got stuck - is there a way I could help them go through? |
@brandon-rhodes I've created PR to @JoshPaterson fork with changes allowing for observe.at to work. One weird issue is that it seems like rows with "E" in uncertainty column screw up propagation for all orbits when passed to Filtering out done using: Other than changes in PR not looking pretty it seems that it should work. Although some testing and refactoring would definetely not hurt. |
Thanks for the update! I have some time this week for open source work, so I'll expect to prioritize this parallel-processing Skkyfield issue. Thanks for reviving it. |
Awesome, there's somewhat related issue for using data form mpcorb.dat files #650 - found out about while trying get precise correlation with telescope observations and comparing differences with orbits pulled from JPL Horizons. That one fortunately would only require adding a paragraph or two to chapter on Kepler orbits as a warning. |
Now that it's getting cold outside I'll have time to devote to this as well! @xmichaelx It'll probably take me a little bit to get back up to speed, then I'll get your PR merged, thanks for making that! |
Hi @brandon-rhodes, @JoshPaterson Is there any progress on this topic of vectorizing the comet objects to enable vectorized calculations on multiple comet objects simultaneously? Best regards |
This commit is only changes that are necessary to vectorize the creation of
_KelperOrbit
objects. This allows theat()
method to work but not the.observe
method. This PR needs to be merged before #526.In order for the tests to pass I need to change them so that they don't use
VectorSum
objects or theobserve
method.