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

REFACTOR: make multiband likelihood call Interferometer.get_detector_response #847

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ColmTalbot
Copy link
Collaborator

See https://git.ligo.org/lscsoft/bilby/-/merge_requests/1379

I think this has a conflict with #842.

Based on ongoing discussion about implementing different detector responses, it would be nice for the likelihoods to all access the response functions uniformly. The MB likelihood currently has an almost verbatim copy of the Inteferometer.get_detector_response method. This MR provides a minimal change to be able to directly call that method.…response

@mj-will
Copy link
Collaborator

mj-will commented Nov 1, 2024

#842 is merged now, so hopefully the conflicts can be resolved.

@ColmTalbot
Copy link
Collaborator Author

This will actually need more extensive changes. I think it would require moving the beam_pattern_reference_time to the Interferometer. I think this would make sense as a change, possibly as another optional argument or an attribute that needs to be manually set, e.g., by the likelihood classes. I'll mark this as draft for now.

@ColmTalbot
Copy link
Collaborator Author

@SMorisaki I can't request a review from you, but please can you review this?

@ColmTalbot ColmTalbot marked this pull request as draft November 14, 2024 15:56
@mj-will mj-will modified the milestones: 2.4.1, 2.5.0 Nov 14, 2024
@mj-will mj-will requested a review from SMorisaki January 15, 2025 14:48
@mj-will
Copy link
Collaborator

mj-will commented Jan 22, 2025

@ColmTalbot @SMorisaki is this in a state where we can get it into the next release?

@mj-will mj-will removed this from the 2.5.0 milestone Jan 23, 2025
@ColmTalbot ColmTalbot marked this pull request as ready for review February 3, 2025 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants