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

GW170817 relative binning example script #49

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

ThibeauWouters
Copy link
Collaborator

@ThibeauWouters ThibeauWouters commented Dec 6, 2023

Example script that is able to run PE for GW170817, addressing #38.

The hyperparameters might not be tuned ideally since these are tested and configured on the external dataset available at the TurboPE repo (i.e. here), and not tested on the data fetched online. Also, the tests were performed with TaylorF2, not IMRPhenomD. But this can serve as initial script to get started, and I have verified that it is able to run without issue.

At the same time, I have also included a small edit in likelihood.py to transpose the bounds variable of maximize_likelihood, since that confused me when working on some scripts. This simple fix might help future users in avoiding getting that confusion.

@kazewong kazewong self-requested a review December 6, 2023 17:53
Regarding this changes. The new styling changes pushed recently should give more explicit hints to address the ambiguity in shape of array. Reverting this in favor of the new changes
@kazewong
Copy link
Owner

kazewong commented Dec 6, 2023

@ThibeauWouters Thanks for the example. It looks good to me.

Regarding the bounds in the heterodyne likelihood. There is a recent merge for styling the code

def __init__(
.

This should give clear enough hints on the shape of array. We can also add an assert before that. Merging for now

@kazewong kazewong merged commit 7e4c75d into kazewong:main Dec 6, 2023
3 of 7 checks passed
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.

2 participants