Skip to content

Fix loglikelihood_function argument in nessai sampler#4860

Open
mj-will wants to merge 1 commit intogwastro:masterfrom
mj-will:fix-nessai-loglikelihood-function
Open

Fix loglikelihood_function argument in nessai sampler#4860
mj-will wants to merge 1 commit intogwastro:masterfrom
mj-will:fix-nessai-loglikelihood-function

Conversation

@mj-will
Copy link
Contributor

@mj-will mj-will commented Aug 29, 2024

Standard information about the request

This is a: bug fix

This change affects: inference

This change changes: scientific output

This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

This change will: fix a bug that prevents an option from being used

Motivation

The current implementation does not allow the user to specify the loglikelihood-function. Instead it raises an error:

RuntimeError: Config contains unknown options: {'loglikelihood-function'}

Contents

This happens because the sampler class checks the specified options against a list of valid options but this did not account for the pycbc-specific option loglikelihood-function. This PR adds loglikelihood-function to the set of options pycbc-specific options that are allowed.

Links to any issues or associated PRs

N/A

Testing performed

I've run the LISA injection example with a modified config where loglikelihood-function=loglr

Additional notes

  • The author of this pull request confirms they will adhere to the code of conduct

@mj-will mj-will added bug BUG - FIX ASAP inference labels Aug 29, 2024
@mj-will mj-will force-pushed the fix-nessai-loglikelihood-function branch from fc4d1f1 to 1124b2b Compare September 5, 2024 07:39
@mj-will
Copy link
Contributor Author

mj-will commented Sep 5, 2024

@HumphreyWang were you able to test this with your problem?

@GarethCabournDavies
Copy link
Contributor

@mj-will what is the status of this?

@mj-will
Copy link
Contributor Author

mj-will commented Jul 4, 2025

@GarethCabournDavies I believe this is working but @HumphreyWang should confirm. If so, then I think it is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug BUG - FIX ASAP inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants