jnarag / teaspoon

A package for sequence analysis of serially-sampled populations
5 stars 1 forks source link

Query about BhattMethod #15

Closed lonelyjoeparker closed 6 years ago

lonelyjoeparker commented 6 years ago

@jnarag in BhattMethod.inferCountsFixedNR() (line:1015 as of 14f17a2) labels finalmat[3][i] as silent/replacement ratio .. but that seems inverted as later it is assigned to replacementSilentRatesRatio?

double[] temp = calculateSiteFrequenciesWithinInterval(binsvalues[0][i], binsvalues[1][i],prior,needPrior);
finalmat[0][i] = temp[0];   // number silentProb, sigma
finalmat[1][i] = temp[1];   // number replacement, rho

totals[0][i] = temp[2];     // total count
totals[1][i] = temp[3];     // total variant
totals[2][i] = temp[4];     // total invariant
if(temp[1]!=0){      // was temp[1] but should be temp[0]
                finalmat[3][i] = temp[1]/temp[0];   // Silent/replacement ratio
} else {
                finalmat[3][i] = Double.NaN;
}

I double checked calcsiteFreqsWithingInterval(), and it definitely returns temp[0] temp[1] as sigma; rho, respectively... am I missing something or is the comment in line 1015 just plain wrong??

jnarag commented 6 years ago

Hi Joe,

Line 1015 is definitely a typo - elsewhere (for a similar method) it states “replacement/silent ratio”. Sorry for the confusion!

Jayna

On 5 Jun 2018, at 12:34, Joe Parker notifications@github.com wrote:

@jnarag https://github.com/jnarag in BhattMethod.inferCountsFixedNR() (line:1015 as of 14f17a2 https://github.com/jnarag/teaspoon/commit/14f17a2a8e0e5bc6f317c170e0c2703b133c2675) labels finalmat[3][i] as silent/replacement ratio .. but that seems inverted as later it is assigned to replacementSilentRatesRatio?

double[] temp = calculateSiteFrequenciesWithinInterval(binsvalues[0][i], binsvalues[1][i],prior,needPrior); finalmat[0][i] = temp[0]; // number silentProb, sigma finalmat[1][i] = temp[1]; // number replacement, rho

totals[0][i] = temp[2]; // total count totals[1][i] = temp[3]; // total variant totals[2][i] = temp[4]; // total invariant if(temp[1]!=0){ // was temp[1] but should be temp[0] finalmat[3][i] = temp[1]/temp[0]; // Silent/replacement ratio } else { finalmat[3][i] = Double.NaN; } I double checked calcsiteFreqsWithingInterval(), and it definitely returns temp[0] temp[1] as sigma; rho, respectively... am I missing something or is the comment in line 1015 just plain wrong??

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jnarag/teaspoon/issues/15, or mute the thread https://github.com/notifications/unsubscribe-auth/ADnVqij2Eul2MxdUiHrD_VAOJneKd1b7ks5t5mzWgaJpZM4Uaphd.

lonelyjoeparker commented 6 years ago

No worries, cheers for clearing that up. Also: just below we have

        int flag=0;
        for(int i=0;i< (int) numBins;i++){
            if(whichBins[i]==false && flag==0){
                this.deleteriousLoad+=getNonNeutralSubstitutions()[i];
            }
            if(whichBins[i]){flag=1;}
            if(whichBins[i]==false && flag==1){
                this.adaptation+=getNonNeutralSubstitutions()[i];
            }
        }

... We only ever seem to pass whichBins ={false,false,false}, and I can't find any other references/accesses to BhattMethod.adaptation or BhattMethod.deleteriousLoad either. Is this just a remnant of something never shipped/implemented, or is it needed?

jnarag commented 6 years ago

I think this was from one of Samir’s thesis chapters. The basic idea is that we can look at non-neutral changes in the low-freq class, which here are assumed to deleterious (while in the high-freq class we assume non-neutral changes are adaptive).

It doesn’t need to be included in the main teaspoon framework, and in theory if folks are interested in this problem they could work this out from the output files (e.g. the neutral ratio and the silent and replacement polymorphisms in the low-free class).

On 5 Jun 2018, at 13:55, Joe Parker notifications@github.com wrote:

No worries, cheers for clearing that up. Also: just below we have

    int flag=0;
    for(int i=0;i< (int) numBins;i++){
        if(whichBins[i]==false && flag==0){
            this.deleteriousLoad+=getNonNeutralSubstitutions()[i];
        }
        if(whichBins[i]){flag=1;}
        if(whichBins[i]==false && flag==1){
            this.adaptation+=getNonNeutralSubstitutions()[i];
        }
    }

... We only ever seem to pass whichBins ={false,false,false}, and I can't find any other references/accesses to BhattMethod.adaptation or BhattMethod.deleteriousLoad either. Is this just a remnant of something never shipped/implemented, or is it needed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jnarag/teaspoon/issues/15#issuecomment-394698378, or mute the thread https://github.com/notifications/unsubscribe-auth/ADnVqsuBs9CdAQYuBf67zxY0xrpz5nR1ks5t5n_MgaJpZM4Uaphd.

lonelyjoeparker commented 6 years ago

OK that makes sense. Think I'll drop it from the new class then - apart from anything else, I couldn't see how BhattMethod.adaptation was ever going to get written to if Nvec elements are always negative...