trvrb / stem

Picking influenza stem strains
http://bedford.io/projects/stem/
0 stars 0 forks source link

Incompatible multivariateTraitLikelihood and antigenicLikelihood #2

Closed trvrb closed 10 years ago

trvrb commented 10 years ago

@rambaut @msuchard ---

There is a bug preventing us from using antigenicLikelihood with a diffusion model that includes drift. Briefly:

Conflict between driftModels and cacheBranches=true

It works to use a diffusion model with drift as specified in stem_diffusion.xml. In this case the cacheBranches in multivariateTraitLikelihood needs to be set to false. Setting cacheBranches="true" results in the following error:

Exception in thread "Thread-1" java.lang.RuntimeException: Unknown componentChangedEvent
    at dr.evomodel.continuous.AbstractMultivariateTraitLikelihood.handleModelChangedEvent(Unknown Source)

This only occurs when driftModels is included in multivariateTraitLikelihood.

Conflict between antigenicLikelihood and cacheBranches=false

It works to use a diffusion model without drift and antigenic MDS as specified in stem_antigenic.xml. However, this requires cacheBranches="true". Otherwise, we get:

SEVERE: State was not correctly restored after reject step.
Likelihood before: -5626.767264971877 Likelihood after: -5626.88559153049
Operator: randomWalkOperator(virusLocations, 1.0, 100.0) virusLocations

I presume this is coming from the update of tip location not propagating properly to multivariateTraitLikelihood.

Issue

Because of these conflicts with cacheBranches, I can't run AntigenicLikelihood with a diffusion model involving drift. Fixing either conflict 1 or 2 would resolve this issue.

msuchard commented 10 years ago

I resolved the first issue via r6033. However, we should probably resolve the second issue as well.

On Nov 25, 2013, at 1:00 PM, Trevor Bedford notifications@github.com wrote:

@rambaut @msuchard ---

There is a bug preventing us from using antigenicLikelihood with a diffusion model that includes drift. Briefly:

Conflict between driftModels and cacheBranches=true

It works to use a diffusion model with drift as specified in stem_diffusion.xml. In this case the cacheBranches in multivariateTraitLikelihood needs to be set to false. Setting cacheBranches="true" results in the following error:

Exception in thread "Thread-1" java.lang.RuntimeException: Unknown componentChangedEvent at dr.evomodel.continuous.AbstractMultivariateTraitLikelihood.handleModelChangedEvent(Unknown Source)

This only occurs when driftModels is included in multivariateTraitLikelihood.

Conflict between antigenicLikelihood and cacheBranches=false

It works to use a diffusion model without drift and antigenic MDS as specified in stem_antigenic.xml. However, this requires cacheBranches="true". Otherwise, we get:

SEVERE: State was not correctly restored after reject step. Likelihood before: -5626.767264971877 Likelihood after: -5626.88559153049 Operator: randomWalkOperator(virusLocations, 1.0, 100.0) virusLocations

I presume this is coming from the update of tip location not propagating properly to multivariateTraitLikelihood.

Issue

Because of these conflicts with cacheBranches, I can't run AntigenicLikelihood with a diffusion model involving drift. Fixing either conflict 1 or 2 would resolve this issue.

— Reply to this email directly or view it on GitHub.

Marc A. Suchard, M.D., Ph.D. Professor Departments of Biomathematics and Human Genetics David Geffen School of Medicine at UCLA, and Department of Biostatistics UCLA School of Public Health 695 Charles E. Young Dr., South 6558 Gonda Los Angeles, CA 90095 310-825-7442 office

trvrb commented 10 years ago

@msuchard Awesome! Thanks so much. I really like the driftModel implementation. Super prescient to do it this way so that things like localClockModel just work without any fuss.

trvrb commented 10 years ago

We have a workaround for the stem project. I've moved the second issue to Google Code. Closing here.

trvrb commented 10 years ago

@msuchard I just noticed that behavior of driftModels vs cacheBranches has been reversed. With update r6033 my test XML stem_diffusion.xml works with cacheBranches="true", but gives:

Exception in thread "Thread-1" java.lang.NullPointerException
    at dr.evomodel.continuous.AbstractMultivariateTraitLikelihood.updateAllNodes(Unknown Source)

when cacheBranches="false". Before r6033, it had worked with cacheBranches set to false and failed with cacheBranches set to true.

This is not a problem for the current project, as cacheBranches="true" works great for antigenicLikelihood.

trvrb commented 10 years ago

Compatibility between driftModels and cacheBranches now complete with Marc's commit r6045.