sblunt / orbitize

Orbit-fitting for directly imaged objects
https://orbitize.info
Other
74 stars 43 forks source link

Fit arbitrary abs astrom #356

Closed sblunt closed 5 months ago

sblunt commented 8 months ago

Two major changes in this pull request, which I'm basing off the hgca branch for now, because both sets of changes touch similar parts of the code.

Change 1: add ability to fit type 1 (stochastic) Hipparcos solutions (for ~ reasons ~) (originally branch fit-type1-hip) Change 2: add the ability to fit arbitrary absolute astrometry (i.e. RA/Dec positions of a star not taken by Hipparcos and Gaia). To do this, I separated out some of the code in hipparcos.py into a new PMPlx_Motion object that is used in fitting the hipparcos IAD but can also be initialized by itself (as is when the user inputs RA/Dec data for body 0).

Note for Jason: I ended up auto-linting a bunch of files, but I'm 95% sure I did linting on separate commits than making actual changes to the code, so hopefully it's not too hard to review. If it is let me know and I can pull out the linting into a separate PR.

Still to do:

sblunt commented 7 months ago

Unit tests added; ready for review.

semaphoreP commented 7 months ago

Should we also be using the PMPlx_Motion class in gaia.py as well? I'm also not sure how PMPlx_Motion is used in the broader orbitize framework for fitting arbitrary absolute astrometry. Can this class be passed into system or something?

Also, the linting makes using the PR "Files changed" interface painful >.<. Commit messages are fine, but not my preferred way of code review. I will survive though.

sblunt commented 6 months ago

Just added a new tutorial that should (hopefully!) make it clearer how this should be used. Also, totally agree that we should refactor gaia.py and the hgca implementation to use the PMPlx_Motion class... maybe a future hack day project lol. How would you feel about pulling this in now and opening an issue for the refactor so we don't forget? XD

semaphoreP commented 5 months ago

Seems like there are conflicts after merging in the dynesty branch.