tempered posterior experiment (Temperature) with log scores

[leokim-l]

Base branch add parameters because I needed that function

[leokim-l]

This method will be implemented after first submission as an alternative method if it ends up yielding better results. We agree @hansenp will implement log scores in BOQA and a prioritiser for his method in Exomiser[boqa branch].

Please leave this branch open as a draft, I will port it where appropriate after Xmas

[leokim-l]

After careful consideration, the tempering of the posterior has been placed inside the BOQA repository, namely in computeBoqaResults, since the default temperature of 1.0 is exactly standard BOQA. The only not too pretty part is the if statement which checks if the temperature is 1.0, then the standard normalization is applied, elsewhere we divide by the largest score (careful, I really mean score, not log(score)). We changed our mind multiple times about where stuff should live, if in Exomiser or in BOQA, I do not believe that it would have made sense to rewrite some of the logic present here in Exomiser's BoqaPrioritiser. Now calling computeBoqaResults with temperature different from 1.0 will change normalization.

Still todo: update javadoc which is probably not correct anymore.

[hansenp]

The re-scaling of the BOQA scores is motivated by the fact that in Exomiser the BOQA scores cannot be combined well with the variant score (and presumably in the future ACMG score), because in most of the cases the BOQA score for the gene/disease at rank 1 is almost 1 and close to zero at all subsequent ranks. Therefore, I believe that such re-scaling methods should be implemented in BoqaPrioritiser.java within Exomiser (where the re-scaling methods we already have are also implemented).

In this PR you created an additional parameter for BOQA (temperature) and embedded it deep within the BOQA code in various places (e.g., AlgorithmParameters, computeUnnormalizedProbability, etc.). This complicates the code without any apparent benefit. So far, we have no evidence that the temperature re-scaling method performs better in the boqa/exomiser context than the re-scaling methods we already have, nor do we have any reason to believe that the temperature re-scaled score offers any advantages over the conventional BOQA probabilities in other contexts. Finally, we still have no idea how to optimally set the additional temperature parameter.

As an alternative to all the changes to BOQA in this PR, one could implement the following function in BoqaPrioritiser.java in Exomiser:

 private static List<BoqaResult> reScaledRawLogTempBoqaExomiserScores(List<BoqaResult> boqaResults, double temperature) {

      // Extract raw BOQA log scores, divide score by temperature and find maximum score
      double maxScore =
              boqaResults.stream()
                      .mapToDouble(r -> r.boqaScore() / temperature)
                      .max()
                      .orElse(Double.NEGATIVE_INFINITY);

      // Re-scale
      return boqaResults.stream()
              .map(br -> {
                  double boqaExomiserScore = Math.exp(br.boqaScore() / temperature - maxScore);
                  return new BoqaResult(br.counts(), boqaExomiserScore);
              })
              .toList();
  }

Let's discuss this next year.

[leokim-l]

Hi, I find myself once more somewhat in disagreement with what you say, sorry! In any case, thanks for the input. I think more scientific questions and "where should the code live" could be handled offline, but since we started, here is my take:

The re-scaling of the BOQA scores is motivated by the fact that in Exomiser the BOQA scores cannot be combined well with the variant score

I agree partially. I think in general, beyond what Exomiser does, there is some interest in having a "decent" distribution of scores. What that means, is not easy to define. In this sense, tempering of posteriors is a standard Bayesian approach, though there is still discussion in the literature about it and I could not make up my mind about how mathematically solid this really is, yet. My gut feeling is that it is a good tool to use.

Therefore, I believe that such re-scaling methods should be implemented in BoqaPrioritiser.java within Exomiser (where the re-scaling methods we already have are also implemented).

Same as answer above, but I would like to further stress that this is not just a pure normalization, but a transformation of the distribution following some principles. It has been picked to be easily merged with Exomiser, but if it turns out working I would not want it do depend on Exomiser, but to be in BOQA where it belongs.

In this PR you created an additional parameter for BOQA (temperature) and embedded it deep within the BOQA code in various places

I don't see an issue with embedding a parameter deep in the codebase, especially if we are in develop. This is git and we are in a development phase, and if it turns out that we do not want it at all, we can still go back. In this regard, I also believe having an extra parameter in AlgorithmParameters is exactly correct and where it should be. We pass AlgorithmParameters as an object and if it contains 2 or 3 parameters, the code using it does not change.

This complicates the code without any apparent benefit. So far, we have no evidence that the temperature re-scaling method performs better in the boqa/exomiser context than the re-scaling methods we already have, nor do we have any reason to believe that the temperature re-scaled score offers any advantages over the conventional BOQA probabilities in other contexts.

I strongly disagree and find this somewhat uncalled for. Temperature is an extension. Temperature equal to 1 behaves exactly in the same way as previous BOQA. Saying we have no evidence that the temperature re-scaling performs better seems a bit unfair. We are in a development phase and the method that is already there is there simply because you put it there, while I had not had the time to put my proposal in until last week. I could even go as far as saying so far, we have no evidence that the re-scaling methods we already have perform better in the boqa/exomiser context than the temperature re-scaling.

I have tried on multiple occasions to explain to you where I think there might be issues with the current method, and it seems to me like you did not even try to understand. I need to put the code somewhere in order to test it, and I would prefer to avoid having the two options, what is already there vs temperature, in two different branches, or with the current way to run exomiser-boqa (recompilation, moving to lib etc.) it will be really impractical to carry out.

Finally, we still have no idea how to optimally set the additional temperature parameter.

This I pointed out myself elsewhere. This is the usual conundrum: adding a parameter makes the model richer, but one also faces the issue of having to find a somewhat robust and detail-independent methodology to tune it. I think, though, that rather than a static frozen re-scaling like the one we already have, which has unpredictable issues depending on the set of diseases one uses and whether some specific patient-disease pair happens to have a very low log-score (simply stated: if a given patient happens to have a really low score among all of the existing diseases) what we really need to have solid, robust results is to have a tunable parameter that can be flexibly adapted if needed. If you don't understand this, I already said I am happy to explain it to you once more, but please refrain from rejecting ideas without having taken the time to try understanding them.

[leokim-l]

The two good points about embedding deep into the codebase are which I fill fix next year:

1. AlgorithmParameters.create() should not only work either without any parameters or with all, but it should be able to work with alpha and beta only.
2. Methods using AlgorithmParameters should not contain explicit parameters in the signature such as here
   

   BOQA/boqa-core/src/main/java/org/p2gx/boqa/core/analysis/BoqaPatientAnalyzer.java

   Line 146 in 82bee1b

   static double computeUnnormalizedProbability(double alpha, double beta, double temperature, BoqaCounts counts){

   
   but rather use the methods such as getAlpha() like here
   

   BOQA/boqa-core/src/main/java/org/p2gx/boqa/core/analysis/BoqaPatientAnalyzer.java

   Line 126 in 82bee1b

   static double computeUnnormalizedLogProbability(AlgorithmParameters params, BoqaCounts counts){

I am not even sure why computeUnnormalizedLogProbability is still here...