Bugfix, filter nulls in getPrimaryTermId

boqa-cli/src/main/java/org/p2gx/boqa/cli/cmd/BoqaBenchmarkCommand.java

@@ -169,7 +169,7 @@ public Integer call() throws Exception {
                     .map(Path::of)
                     .parallel()
                     .map(singleFile -> {
-                        PatientData ppkt = new PhenopacketData(singleFile);
+                        PatientData ppkt = new PhenopacketData(singleFile, hpo);
                         BoqaAnalysisResult result = BoqaPatientAnalyzer.computeBoqaResults(
                                 ppkt, counter, limit, params);
                         int count = fileCount.incrementAndGet();

boqa-core/src/main/java/org/p2gx/boqa/core/internal/OntologyTraverser.java

@@ -60,7 +60,7 @@ public class OntologyTraverser {
     private static final Set<TermId> LOGGED_REPLACEMENTS = ConcurrentHashMap.newKeySet();
     private static final TermId PHENOTYPIC_ABNORMALITY = TermId.of("HP:0000118");
 
-    private static Ontology hpo = null;
+    private final Ontology hpo;
     private final OntologyGraph<TermId> hpoGraph;
     private final Cache<TermId, Collection<TermId>> hpoAncestorsCache = Caffeine.newBuilder().maximumSize(500).build();
 
@@ -71,13 +71,16 @@ public class OntologyTraverser {
      * @todo .extractSubgraph(PHENOTYPIC_ABNORMALITY) or .subOntology(PHENOTYPIC_ABNORMALITY) in phenol don't seem to work.
      */
     public OntologyTraverser(Ontology hpo) {
-        OntologyTraverser.hpo = hpo;
-        hpoGraph = OntologyTraverser.hpo.graph();
+        this.hpo = hpo;
+        this.hpoGraph = hpo.graph();
     }
 
     public OntologyGraph<TermId> getHpoGraph() {
         return hpoGraph;
     }
+    public Ontology getHpoOntology() {
+        return hpo;
+    }
 
     /**
      * Given a set of HPO terms, computes and returns the set of all implied terms, included those passed as input.
@@ -119,10 +122,13 @@ public Set<TermId> initLayer(Set<TermId> hpoTerms) {
      * @param t the HPO term to resolve
      * @return the primary TermId corresponding to the input term
      */
-    public static TermId getPrimaryTermId(TermId t){
+    public TermId getPrimaryTermId(TermId t){
         TermId primaryTermId = hpo.getPrimaryTermId(t);
         if (primaryTermId == null) {
-            LOGGER.warn("Invalid HPO term {}! Skipping...", primaryTermId);
+            if (LOGGED_REPLACEMENTS.add(t)) { // only log once per term
+                LOGGER.warn("Invalid HPO term {}! Skipping it, this may negatively affect performance. " +
+                        "Are you using the latest HPO build? Consider running `download -w` to fetch the latest HPO release.", t);
+            }
         } else {
             if (!t.equals(primaryTermId) && LOGGED_REPLACEMENTS.add(t)) {
                 LOGGER.info("Replacing {} with primary term {}", t, primaryTermId);

boqa-core/src/main/java/org/p2gx/boqa/core/patient/PhenopacketData.java

@@ -1,6 +1,7 @@
 package org.p2gx.boqa.core.patient;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import org.monarchinitiative.phenol.ontology.data.Ontology;
 import org.p2gx.boqa.core.PatientData;
 import org.monarchinitiative.phenol.ontology.data.TermId;
 import org.p2gx.boqa.core.internal.OntologyTraverser;
@@ -12,6 +13,7 @@
 
 import java.nio.file.Path;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -41,14 +43,16 @@ public class PhenopacketData implements PatientData {
     record DiseaseDTO(String id, String label) {}
 
     // Primary constructor
-    public PhenopacketData(Phenopacket phenopacket) {
+    public PhenopacketData(Phenopacket phenopacket, Ontology hpo) {
         this.ppktId = phenopacket.getId();
+        OntologyTraverser traverser = new OntologyTraverser(hpo);
         this.observedTerms =  phenopacket.getPhenotypicFeaturesList().stream()
                 .filter(Predicate.not(PhenotypicFeature::getExcluded))
                 .map(PhenotypicFeature::getType)
                 .map(OntologyClass::getId)
                 .map(TermId::of)
-                .map(OntologyTraverser::getPrimaryTermId)
+                .map(traverser::getPrimaryTermId)
+                .filter(Objects::nonNull) // If old HPO is used without a term, avoids the program crashing
                 .collect(Collectors.toSet());
         if (this.observedTerms.isEmpty()) {
             LOGGER.warn("Phenopacket {} has no observed phenotypic features!", phenopacket.getId());
@@ -57,15 +61,16 @@ public PhenopacketData(Phenopacket phenopacket) {
                 .map(PhenotypicFeature::getType)
                 .map(OntologyClass::getId)
                 .map(TermId::of)
-                .map(OntologyTraverser::getPrimaryTermId)
+                .map(traverser::getPrimaryTermId)
+                .filter(Objects::nonNull) // If old HPO is used without a term, avoids the program crashing
                 .collect(Collectors.toSet());
         this.diseases = phenopacket.getDiseasesList().stream().map(d ->
                 new DiseaseDTO(d.getTerm().getId(), d.getTerm().getLabel())).toList();
     }
 
     // Convenience constructor (allow from file)
-    public PhenopacketData(Path phenopacketFile) {
-        this(PhenopacketReader.readPhenopacket(phenopacketFile));
+    public PhenopacketData(Path phenopacketFile, Ontology hpo) {
+        this(PhenopacketReader.readPhenopacket(phenopacketFile), hpo);
     }
 
     @JsonProperty("diagnosis")

boqa-core/src/test/java/org/p2gx/boqa/core/algorithm/BoqaSetCounterTest.java

@@ -24,25 +24,26 @@
 import static org.p2gx.boqa.core.analysis.BoqaPatientAnalyzer.computeBoqaResults;
 import static org.junit.jupiter.api.Assertions.*;
 
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class BoqaSetCounterTest {
 
-    private static DiseaseData diseaseData;
-    private static Counter counter;
+    private DiseaseData diseaseData;
+    private Counter counter;
+    private Ontology hpo;
 
     @BeforeAll
-    static void setup() throws IOException {
+    void setup() throws IOException {
         try (InputStream annotationStream = new GZIPInputStream(BoqaSetCounterTest.class
                 .getResourceAsStream("/org/p2gx/boqa/core/phenotype.v2025-05-06.hpoa.gz"))) {
-            diseaseData = DiseaseDataParser.parseDiseaseDataFromHpoa(annotationStream);
+            this.diseaseData = DiseaseDataParser.parseDiseaseDataFromHpoa(annotationStream);
         }
-        Ontology hpo;
         try (
             InputStream ontologyStream = new GZIPInputStream(Objects.requireNonNull(OntologyTraverserTest.class
                     .getResourceAsStream("/org/p2gx/boqa/core/hp.v2025-05-06.json.gz")))
         ) {
-            hpo = OntologyLoader.loadOntology(ontologyStream);
+            this.hpo = OntologyLoader.loadOntology(ontologyStream);
         }
-        counter = new BoqaSetCounter(diseaseData, hpo);
+        this.counter = new BoqaSetCounter(diseaseData, hpo);
     }
 
     @Tag("expensive_test")
@@ -105,7 +106,7 @@ void testComputeBoqaCountsAgainstPyboqa(
         int limit =  Integer.MAX_VALUE;
         AlgorithmParameters params = AlgorithmParameters.create(0.2,0.3); // numbers don't matter
         BoqaAnalysisResult boqaAnalysisResult = computeBoqaResults(
-                new PhenopacketData(ppkt), counter, limit, params);
+                new PhenopacketData(ppkt, hpo), counter, limit, params);
 
         BoqaCounts match = null;
         for (BoqaResult br : boqaAnalysisResult.boqaResults()){

boqa-core/src/test/java/org/p2gx/boqa/core/internal/OntologyTraverserTest.java

@@ -24,7 +24,6 @@ public class OntologyTraverserTest {
 
     OntologyTraverser ontologyTraverser;
 
-    // TODO: Daniel suggests using Extensions API rather then TestBase and extensions thereof, more modern.
     @BeforeAll
     void setUp() throws IOException {
         try (
@@ -67,12 +66,13 @@ private static Stream<Arguments> activeLayers(){
     @ParameterizedTest(name = "{0}")
     @MethodSource("oldTerms")
     void testgetPrimaryTermId(String testName, Set<String> expectedNodes, Set<TermId> observedNodes ) {
+
         Set<TermId> expectedNodesTermIds = expectedNodes.stream()
                 .map(TermId::of)
-                .map(OntologyTraverser::getPrimaryTermId)
+                .map(ontologyTraverser::getPrimaryTermId)
                 .collect(Collectors.toSet());
         Set<TermId> observedNodesTermIds = observedNodes.stream()
-                .map(OntologyTraverser::getPrimaryTermId)
+                .map(ontologyTraverser::getPrimaryTermId)
                 .collect(Collectors.toSet());
         assertEquals(expectedNodesTermIds, ontologyTraverser.initLayer(observedNodesTermIds));
     }

boqa-core/src/test/java/org/p2gx/boqa/core/patient/PhenopacketDataTest.java

@@ -55,7 +55,7 @@ void setUp() throws IOException {
                     throw new IOException("Resource not found: " + filename);
                 }
                 Path ppkt = Path.of(resourceUrl.toURI());
-                examplePpkts.add(new PhenopacketData(ppkt));
+                examplePpkts.add(new PhenopacketData(ppkt, ontologyTraverser.getHpoOntology()));
             } catch (URISyntaxException e) {
                 throw new IOException("Failed to resolve resource URI", e);
             }