-t/--threshold silently ignored

[justaddcoffee]

To reproduce:

java -jar target/LIRICAL.jar phenopacket -p one_HPO.json -t 90 or (not clear from docs whether -t should be a fraction or percentage): java -jar target/LIRICAL.jar phenopacket -p one_HPO.json -t 0.90 with this phenopacket:

{
  "subject": {
    "id": "simple_json"
  },
  "phenotypicFeatures": [{
      "type": {
        "id": "HP:0012735"
      }
    }]

This produces output for many diff diagnoses with < 90% posterior probability:

[justaddcoffee]

A related issue is that the we need to check here: https://github.com/TheJacksonLaboratory/LIRICAL/blob/63dcae458c9f0dc4cad5bb2cf51f8037a104fb63/src/main/resources/liricalHTML.ftl#L404 that sparkline is not empty or we'll crash at runtime. I was getting this error at some point but can't reproduce it ATM because of the -t bug in this ticket

[pnrobinson]

Line .78 in HtmlTemplate is checking the threshold, which should be a floating point number. There is also a MIN_DIAGNOSES_TO_SHOW option mainly to avoid the situation when no diagnosis has an above-threshold probability --- it seems better to show detailed results at least for the top 3, or 5, or 10 or whatever. Still there might well be a bug in the logic, maybe we can pair-program this or do you see the error...?

[justaddcoffee]

Sure, glad to pair program today.

I think the -t flag should trump the MIN_DIAGNOSES_TO_SHOW - so for example in this ticket, java -jar target/LIRICAL.jar phenopacket -p one_HPO.json -t 0.90 should not output info for any of the above diseases, since this output directly contradicts the -t flag.

[pnrobinson]

After discussion with Justin we thought the best thing is to make the -m and the -t options be mutually incompatible. Also, the program should not accept a -t option unless it is [0,1]. I will implement this.

[pnrobinson]

I addressed this here https://github.com/TheJacksonLaboratory/LIRICAL/pull/479 running out of mental steam but I think it's ok.

[pnrobinson]

this is addressed, I think.