Skip to content

fix hyperprior in beauti #157 - #167

Merged
rbouckaert merged 1 commit into
masterfrom
hyperprior
Sep 10, 2026
Merged

rbouckaert merged 1 commit into
masterfrom
hyperprior

Conversation

@walterxie

@walterxie walterxie commented Sep 7, 2026 •

Copy link
Copy Markdown
Member

Fix #157.

In the Priors panel, checking estimate on a distribution's own parameter (e.g. Normal's mean/sigma, LogNormal's M/S) is supposed to pop up a "Hyper prior" confirmation dialog and, once confirmed, add a new hyperprior row to the panel. This silently did nothing.

Root cause

Two separate bugs, both in ScalarInputEditor.toggleEstimate():

  1. The code checked id.startsWith("RealParameter") to decide whether the parameter needed renaming before a hyperprior could be created. That check is a leftover from the old (pre-spec-framework) parameter class — current parameters auto-generate ids like RealScalarParam.7, which never matches, so the rename step (and everything after it) never ran.
  2. Even with that fixed, the underlying hyperprior template itself was broken for current-style parameters: it wrapped a legacy Prior object around a OneOnX distribution, but Prior.x requires the old Function interface, which today's parameters don't implement. So creating the hyperprior would have failed anyway.

Both failures were silently swallowed by a catch-all in toggleEstimate(), which is why the checkbox appeared to just do nothing.

Fix

  • ScalarInputEditor.toggleEstimate(): replaced the id-string check with a structural one — look for a distribution among the parameter's own outputs whose param already points back at it. This is a direct fact about the object graph rather than a fragile naming convention, so a renamed id or an unrelated object can't be mistaken for an existing hyperprior.
  • BeautiConfig.HYPER_PRIOR_XML: the hyperprior template now builds a LogUniform distribution directly (matching the same "LogUniform replaces OneOnX" change already made elsewhere in ParametricDistributions.xml), instead of the legacy Prior+OneOnX combination that no longer works with current parameter types.
  • Confirmed the checkbox still does not pop up the hyperprior dialog for Site Model / Clock Model parameters (that gating already existed and is untouched).

Testing

Added HyperPriorTest, a GUI test driving the actual Priors panel:

  • checking estimate on LogNormal's M and S correctly adds a hyperprior row for each, wired to the right parameter
  • checking estimate in the Site Model / Clock Model panels never triggers the hyperprior dialog

All 16 tests in the affected area pass (HyperPriorTest, FixedMeanRateTest, ScalarInputEditorTest, ScalarDistributionInputEditorTest, TensorDistributionInputEditorTest).

@walterxie
walterxie requested a review from rbouckaert September 7, 2026 03:37
@walterxie

Copy link
Copy Markdown
Member Author
Screenshot 2026-09-07 at 15 45 55

@rbouckaert
rbouckaert merged commit 05d5b13 into master Sep 10, 2026
1 check 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.

Priors panel: "estimate" checkbox missing, and hyper-prior creation broken

2 participants