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

Check SOURCE_ID and source_id #56

Merged
merged 4 commits into from
Aug 5, 2024
Merged

Conversation

tomasstolker
Copy link
Contributor

@tomasstolker tomasstolker commented Jul 29, 2024

  • The column of the Gaia source ID was changed from source_id to SOURCE_ID so both are now checked
  • The gaia_epoch attribute needed to be set when reading from a query_file
  • Fixed an issue related to the number of dimensions when plotting the posterior

@gotten
Copy link
Collaborator

gotten commented Jul 30, 2024

I get the same error as the CI on a fresh install of your PR.

@gotten
Copy link
Collaborator

gotten commented Jul 30, 2024

ndim also includes the parameters of the host star. so it is bigger than the amount of labels. so the if statement that checks the dims and appends a label uses a value that wasnt expected.

ndim is a weird leftover from the way we ran the initial code.

if i recall

  • the host star can be fixed at median coordinates or used with the gaia measurements as prior (also with or without RV prior) 0, 5, 6 dims
  • the background star can be fit with ra dec and propermotions and fixed at infinity or the parallax is fit with a realistic prior based on the original Gaia DR3 priors (another 4 or 5 dims)

a robust fix is to revamp the ndim system. for now we need to change the number in the if statement that appends a parallax to the labels. I think we kind of assume in the end that we use "11" dimensions in almost all fits. I'm running a test now with ndim==4 replaced by ndim ==10 and ndim==5 replaced by ndim==11.

@tomasstolker
Copy link
Contributor Author

Right, I agree, it requires a more complete implementation! Let me try to fix this case and I will leave a more robust implementation for someone else, since I don't know the code sufficiently well for that.

@gotten
Copy link
Collaborator

gotten commented Jul 30, 2024

Still crashes?. So label is defined by us as something that is 4 or 5 long. And cornerplot looks at the size of samples (extracted from dynesty results) for the size of the for loop that also goes over the labels. So there can be a mismatch there.

@tomasstolker tomasstolker changed the title Check SOURCE_ID and source_id, dimension fix in plot function Check SOURCE_ID and source_id Jul 30, 2024
@tomasstolker
Copy link
Contributor Author

I couldn't find an easy fix, so reverted those changes!

@wbalmer wbalmer merged commit 34218b7 into wbalmer:main Aug 5, 2024
3 checks passed
@tomasstolker tomasstolker deleted the source_id branch August 19, 2024 06:52
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.

3 participants