Skip to content
Snippets Groups Projects

Fix LALInference Gaussian Mixture Model prior for multiple values

Merged Matthew Pitkin requested to merge matthew-pitkin/lalsuite:gmm_prior_varnames into master
All threads resolved!

Description

The Gaussian Mixture Model (GMM) prior in LALInferencePrior.c contains a bug that means that if there is more than one parameter being used, and values are drawn outside the prior ranges, the values get stuck and never change. This MR fixes this by making sure that previous values get removed if the value is outside the bounds. It also fixes another minor bug in the setting of a string variable.

The MR also makes a minor change to the ppe_likelihood code used when setting up a GMM from a prior file. It now check if the lower and upper bounds are finite independently of each other, rather than requiring both to be finite.

Notes: I have made the VARNAME_MAX macro in LALInference.h bigger, changing it from 40 to 50. This is to accommodate more GMM variables. If this might be problematic for any other LALInference code I can remove this.

API Changes and Justification

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

If any of the Backwards Incompatible check boxes are ticked please provide a justification why this change is necessary and why it needs to be done in a backwards incompatible way.

Review Status

I have verified that the code compiles and works for a pulsar parameter estimation example. As this touches both LALInference and lalapps/pulsar code cc @vivien, @john-veitch, @david-keitel, @karl-wette.

Merge request reports

Pipeline #115131 passed

Pipeline passed for c3b71128 on matthew-pitkin:gmm_prior_varnames

Approval is optional

Merged by Karl WetteKarl Wette 4 years ago (Apr 1, 2020 4:12am UTC)

Merge details

  • Changes merged into master with 60bb1b25.
  • Deleted the source branch.

Pipeline #115185 passed

Pipeline passed for 60bb1b25 on master

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Please register or sign in to reply
Loading