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

Fix UPP distribution #407

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

Fix UPP distribution #407

wants to merge 3 commits into from

Conversation

hoehna
Copy link
Member

@hoehna hoehna commented Nov 2, 2023

This (hopefully) fixes the issue #293

We should be propagating the call to touch if the parameter values change. Thus, we need to implement:

  • getAffected
  • keepSpecialization
  • restoreSpecialization
  • touchSpecialization

@hoehna hoehna changed the base branch from master to development November 2, 2023 08:58
@bredelings
Copy link
Contributor

bredelings commented Nov 2, 2023

Thanks! I will look at this more carefully later.

I did a quick check, and we still get:

   Error:	Issue in 'UPP-Allocation' on 'rel_branch_rates[106]' after move because posterior of -529.952 and -529.957/-529.957. The move was rejected.

So #293 is not fixed yet... although the PR could be fine. I will try and do a bit more investigation.

@bredelings
Copy link
Contributor

bredelings commented Nov 7, 2023

One issue is that the mvUPPAllocation does not seem to initiate any touching. It just does dist.redrawValue():

And that just does this->value = simulate():

But perhaps I am missing something.

@davidcerny
Copy link
Contributor

Unfortunately, this still fails on the case from #293 with the same error:

   Running MCMC simulation
   This simulation runs 1 independent replicate.
   The simulator uses 823 different moves in a random move schedule with 2139 moves per iteration
   

Iter        |      Posterior   |     Likelihood   |          Prior   |   num_samp_anc   |    origin_time   |    elapsed   |        ETA   |
------------------------------------------------------------------------------------------------------------------------------------------
0           |       -8414.05   |       -7135.28   |       -1278.77   |              0   |       250.4522   |   00:00:00   |   --:--:--   |
   Error:	Issue in 'UPP-Allocation' on 'rel_branch_rates[74]' after move because posterior of -5646.09 and -5646.35/-5646.35. The move was rejected.
   Error:	Problem processing line 500 in file "/Users/David/Downloads/mvSlice_test/mvSlice_test_rjMCMC_over_partitions.Rev"

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.

None yet

3 participants