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

Relax requirements on coal_times #2487

Merged
merged 1 commit into from
May 16, 2020
Merged

Relax requirements on coal_times #2487

merged 1 commit into from
May 16, 2020

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented May 16, 2020

Addresses #2426
Blocking #2468

This relaxes two restrictions on CoalescentLikelihood that proved inconvenient in #2426:

  1. coal_times and leaf_times are no longer required to lie in [0, duration]. Previously the CoalescentTimes* distributions allowed times outside of [0,duration], but CoalescentLikelihood required values in [0,duration]. And it turns out real data shows coal_times before the start of simulation.
  2. CoalescentLikelihood no longer requires sorted coal_times. This made sense for the CoalescentTimes* distributions for normalization, but is not needed for the likelihood.

Tested

  • added unit tests
  • randomly permuted coal_times in an SEIR smoke test

@eb8680 eb8680 merged commit ec8a4a0 into dev May 16, 2020
@eb8680 eb8680 deleted the coalescent-relax branch May 16, 2020 18:59
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