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

Fit arbitrary abs astrom #356

Merged
merged 39 commits into from
Apr 11, 2024
Merged

Fit arbitrary abs astrom #356

merged 39 commits into from
Apr 11, 2024

Conversation

sblunt
Copy link
Owner

@sblunt sblunt commented Jan 19, 2024

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:

  • add unit tests for both major changes

Base automatically changed from hcga to v3 January 19, 2024 02:02
@sblunt
Copy link
Owner Author

sblunt commented Jan 30, 2024

Unit tests added; ready for review.

@semaphoreP
Copy link
Collaborator

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
Copy link
Owner Author

sblunt commented Mar 13, 2024

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

Copy link
Collaborator

@semaphoreP semaphoreP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like the format.

@semaphoreP
Copy link
Collaborator

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

@sblunt sblunt changed the base branch from v3 to obs-priors April 11, 2024 18:54
Base automatically changed from obs-priors to v3 April 11, 2024 22:43
@sblunt sblunt merged commit 5afbc1a into v3 Apr 11, 2024
7 of 8 checks passed
@sblunt sblunt deleted the fit-arbitrary-abs-astrom branch April 11, 2024 22:47
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