From 323f98bd72f8dcb9cc9d16794ed72f75f5840723 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 13:27:00 -0400 Subject: [PATCH 1/9] Setup benchmark stubs. --- phenol-io/pom.xml | 14 +++++ .../phenol/io/benches/Constants.java | 10 +++ .../phenol/io/benches/OntologyGraphBench.java | 27 ++++++++ .../io/benches/PhenolOntologySetup.java | 39 ++++++++++++ .../phenol/io/benches/SetupHpo.java | 61 +++++++++++++++++++ pom.xml | 13 ++++ 6 files changed, 164 insertions(+) create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/Constants.java create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/SetupHpo.java diff --git a/phenol-io/pom.xml b/phenol-io/pom.xml index 96cf340f2..f4ad0f4f0 100644 --- a/phenol-io/pom.xml +++ b/phenol-io/pom.xml @@ -28,6 +28,20 @@ org.yaml snakeyaml + + org.openjdk.jmh + jmh-core + + + org.openjdk.jmh + jmh-generator-annprocess + + + org.monarchinitiative.biodownload + biodownload + 1.1.0 + test + diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/Constants.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/Constants.java new file mode 100644 index 000000000..c72e8e74a --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/Constants.java @@ -0,0 +1,10 @@ +package org.monarchinitiative.phenol.io.benches; + +/** + * Constants to use across all benchmarks. + */ +class Constants { + + static final String HPO_VERSION = "v2023-10-09"; + +} diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java new file mode 100644 index 000000000..762b5f470 --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java @@ -0,0 +1,27 @@ +package org.monarchinitiative.phenol.io.benches; + +import org.monarchinitiative.phenol.ontology.data.TermId; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Benchmark different implementations of {@link org.monarchinitiative.phenol.graph.OntologyGraph}. + */ +@BenchmarkMode(Mode.Throughput) +public class OntologyGraphBench { + + @Benchmark + public void phenol_getParents(PhenolOntologySetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + blackhole.consume(ontology.ontology.graph().getParents(termId, false)); + } + } + + @Benchmark + public void phenol_getChildren(PhenolOntologySetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + blackhole.consume(ontology.ontology.graph().getChildren(termId, false)); + } + } + +} diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java new file mode 100644 index 000000000..b87c710e0 --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java @@ -0,0 +1,39 @@ +package org.monarchinitiative.phenol.io.benches; + +import org.monarchinitiative.phenol.io.MinimalOntologyLoader; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.monarchinitiative.phenol.ontology.data.Term; +import org.monarchinitiative.phenol.ontology.data.TermId; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; +import java.util.Random; +import java.util.stream.Collectors; + + +@State(Scope.Thread) +public class PhenolOntologySetup { + + private static final Random RANDOM = new Random(500); + + MinimalOntology ontology; + List primaryTermIds; + + @Setup(Level.Trial) + public void prepare() { + Path hpoPath = SetupHpo.hpoJsonPath(Constants.HPO_VERSION); + ontology = MinimalOntologyLoader.loadOntology(hpoPath.toFile()); + primaryTermIds = ontology.getTerms().stream() + .map(Term::id) + .collect(Collectors.toList()); + + // ensure the primary term IDs are not sorted if that's what an ontology does. + Collections.shuffle(primaryTermIds, RANDOM); + } + +} diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/SetupHpo.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/SetupHpo.java new file mode 100644 index 000000000..4f02d8b87 --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/SetupHpo.java @@ -0,0 +1,61 @@ +package org.monarchinitiative.phenol.io.benches; + +import org.monarchinitiative.biodownload.BioDownloader; +import org.monarchinitiative.biodownload.FileDownloadException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Path; + +/** + * Helper for downloading a specific version of HPO into a platform-specific tmp directory. + *

+ * The HPO JSON is not overwritten if it already exists. + */ +class SetupHpo { + + private static final Logger LOGGER = LoggerFactory.getLogger(SetupHpo.class); + + /** + * Download a JSON file of given HPO version into a tmp folder. + * + * @param version the version to download, e.g. 

v2023-10-09
+ * @return path to the downloaded HPO JSON file. + */ + static Path hpoJsonPath(String version) { + LOGGER.info("Preparing HPO URL for version {}", version); + URL hpoUrl = prepareHpoUrl(version); + LOGGER.info("Using HPO from {}", hpoUrl); + String hpJsonName = String.format("hp.%s.json", version); + + + // Setup builder + Path tmpDir = Path.of(System.getProperty("java.io.tmpdir")).resolve("phenol-bench"); + LOGGER.info("Downloading HPO to {}", tmpDir); + BioDownloader downloader = BioDownloader.builder(tmpDir) + .custom(hpJsonName, hpoUrl) + .build(); + + // Download + try { + downloader.download(); + } catch (FileDownloadException e) { + throw new RuntimeException(e); + } + + return tmpDir.resolve(hpJsonName); + } + + private static URL prepareHpoUrl(String version) { + try { + return new URI(String.format("https://github.com/obophenotype/human-phenotype-ontology/releases/download/%s/hp.json", version)).toURL(); + } catch (URISyntaxException | MalformedURLException e) { + throw new RuntimeException(e); + } + } + +} diff --git a/pom.xml b/pom.xml index a0ca49cab..d476b1470 100644 --- a/pom.xml +++ b/pom.xml @@ -33,6 +33,7 @@ 2.2 5.9.3 5.3.1 + 1.37 @@ -209,6 +210,18 @@ ${hamcrest.version} test + + org.openjdk.jmh + jmh-core + ${jmh.version} + test + + + org.openjdk.jmh + jmh-generator-annprocess + ${jmh.version} + test + From a8de4e1f3e820185eb2c66b146112c13f4e31412 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 13:47:30 -0400 Subject: [PATCH 2/9] Refactor `CsrOntologyGraph` to `CsrPolyOntologyGraph`. --- .../phenol/graph/OntologyGraphBuilders.java | 4 +- .../phenol/graph/csr/CsrData.java | 10 ++--- .../phenol/graph/csr/Util.java | 26 ----------- .../phenol/graph/csr/package-info.java | 6 +-- .../CsrPolyOntologyGraph.java} | 27 +++++++----- .../CsrPolyOntologyGraphBuilder.java} | 24 +++++----- .../graph/csr/{ => poly}/CsrRowBuilder.java | 2 +- .../graph/csr/{ => poly}/DataIndexer.java | 10 ++--- .../{ => poly}/InfallibleMappingIterator.java | 2 +- .../InsufficientWidthException.java | 2 +- .../csr/{ => poly}/NodeIndexIterator.java | 2 +- .../graph/csr/{ => poly}/RelationCodec.java | 2 +- .../csr/{ => poly}/RelationCodecDefault.java | 2 +- .../StaticCsrArray.java} | 40 +++++------------ .../{ => poly}/indexer/DataIndexerByte.java | 4 +- .../indexer/DataIndexerInteger.java | 4 +- .../{ => poly}/indexer/DataIndexerLong.java | 4 +- .../{ => poly}/indexer/DataIndexerShort.java | 4 +- .../csr/{ => poly}/indexer/package-info.java | 2 +- .../phenol/graph/csr/poly/package-info.java | 8 ++++ .../phenol/graph/csr/util/Util.java | 44 +++++++++++++++++++ .../phenol/graph/csr/util/package-info.java | 4 ++ .../phenol/graph/BaseOntologyGraphTest.java | 12 ++--- .../phenol/graph/csr/CsrDatasets.java | 10 ++--- .../CsrPolyOntologyGraphBuilderTest.java} | 22 +++++----- .../CsrPolyOntologyGraphTest.java} | 4 +- .../csr/{ => poly}/CsrRowBuilderTest.java | 2 +- .../graph/csr/{ => poly}/DataIndexerTest.java | 2 +- .../{ => poly}/RelationCodecDefaultTest.java | 6 +-- .../StaticCsrArrayTest.java} | 30 +++++++------ 30 files changed, 169 insertions(+), 152 deletions(-) delete mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/Util.java rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{CsrOntologyGraph.java => poly/CsrPolyOntologyGraph.java} (87%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{CsrOntologyGraphBuilder.java => poly/CsrPolyOntologyGraphBuilder.java} (85%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/CsrRowBuilder.java (97%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/DataIndexer.java (81%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/InfallibleMappingIterator.java (94%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/InsufficientWidthException.java (94%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/NodeIndexIterator.java (94%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/RelationCodec.java (93%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/RelationCodecDefault.java (97%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ImmutableCsrMatrix.java => poly/StaticCsrArray.java} (67%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/indexer/DataIndexerByte.java (83%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/indexer/DataIndexerInteger.java (83%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/indexer/DataIndexerLong.java (83%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/indexer/DataIndexerShort.java (83%) rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/indexer/package-info.java (87%) create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/package-info.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/package-info.java rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{CsrOntologyGraphBuilderTest.java => poly/CsrPolyOntologyGraphBuilderTest.java} (84%) rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{CsrOntologyGraphTest.java => poly/CsrPolyOntologyGraphTest.java} (98%) rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/CsrRowBuilderTest.java (96%) rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/DataIndexerTest.java (98%) rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/RelationCodecDefaultTest.java (95%) rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{ImmutableCsrMatrixTest.java => poly/StaticCsrArrayTest.java} (83%) diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java index c8491c1c4..0802de62b 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java @@ -1,7 +1,7 @@ package org.monarchinitiative.phenol.graph; -import org.monarchinitiative.phenol.graph.csr.CsrOntologyGraphBuilder; +import org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraphBuilder; import org.monarchinitiative.phenol.ontology.data.TermId; /** @@ -31,7 +31,7 @@ private OntologyGraphBuilders(){} * */ public static OntologyGraphBuilder csrBuilder(Class clz) { - return CsrOntologyGraphBuilder.builder(clz); + return CsrPolyOntologyGraphBuilder.builder(clz); } } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java index 041badaf6..113158f2d 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java @@ -5,27 +5,27 @@ * @param data type * @author Daniel Danis */ -class CsrData { +public class CsrData { private final int[] indptr; private final int[] indices; private final T[] data; - CsrData(int[] indptr, int[] indices, T[] data) { + public CsrData(int[] indptr, int[] indices, T[] data) { this.indptr = indptr; this.indices = indices; this.data = data; } - int[] getIndptr() { + public int[] getIndptr() { return indptr; } - int[] getIndices() { + public int[] getIndices() { return indices; } - T[] getData() { + public T[] getData() { return data; } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/Util.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/Util.java deleted file mode 100644 index dfcfe1db5..000000000 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/Util.java +++ /dev/null @@ -1,26 +0,0 @@ -package org.monarchinitiative.phenol.graph.csr; - -import org.monarchinitiative.phenol.graph.NodeNotPresentInGraphException; - -import java.util.Arrays; -import java.util.Comparator; -import java.util.List; - -class Util { - private Util(){} - - static int getIndexOfUsingBinarySearch(T node, T[] nodes, Comparator comparator) { - int idx = Arrays.binarySearch(nodes, node, comparator); - if (idx >= 0) - return idx; - else - throw new NodeNotPresentInGraphException(String.format("Item not found in the graph: %s", node)); - } - - static int[] toIntArray(List indices) { - int[] a = new int[indices.size()]; - for (int i = 0; i < indices.size(); i++) - a[i] = indices.get(i); - return a; - } -} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java index 9ad31f379..8734e1eca 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java @@ -1,8 +1,8 @@ /** * A package with CSR implementation of {@link org.monarchinitiative.phenol.graph.OntologyGraph}. * - * @see org.monarchinitiative.phenol.graph.csr.CsrOntologyGraph - * @see org.monarchinitiative.phenol.graph.csr.CsrOntologyGraphBuilder - * @see org.monarchinitiative.phenol.graph.csr.ImmutableCsrMatrix + * @see org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph + * @see org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraphBuilder + * @see org.monarchinitiative.phenol.graph.csr.poly.StaticCsrArray */ package org.monarchinitiative.phenol.graph.csr; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraph.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java similarity index 87% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraph.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java index 6146ab084..a0736c3cb 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraph.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java @@ -1,6 +1,8 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.monarchinitiative.phenol.graph.OntologyGraph; +import org.monarchinitiative.phenol.graph.csr.ItemsNotSortedException; +import org.monarchinitiative.phenol.graph.csr.util.Util; import org.monarchinitiative.phenol.utils.IterableIteratorWrapper; import java.util.*; @@ -8,32 +10,33 @@ import java.util.stream.Stream; /** - * {@link OntologyGraph} backed by an adjacency matrix in CSR format. + * An {@link OntologyGraph} backed by an adjacency matrix in CSR format. + *

+ * Besides implementing {@linkplain OntologyGraph}, the class supports storing graphs with multiple edge types. *

* The traversals are implemented using iterators to prevent unnecessary allocation. *

- * * The array of nodes must be sorted and no duplicate elements must be present. An exception is thrown otherwise. * * @param node type. * @param data type for storing the relationships between graph nodes. * @author Daniel Danis */ -public class CsrOntologyGraph implements OntologyGraph { +public class CsrPolyOntologyGraph implements OntologyGraph { private final T root; private final T[] nodes; - private final ImmutableCsrMatrix adjacencyMatrix; + private final StaticCsrArray adjacencyMatrix; private final Comparator comparator; private final Predicate hierarchy; private final Predicate hierarchyInverted; - public CsrOntologyGraph(T root, - T[] nodes, - ImmutableCsrMatrix adjacencyMatrix, - Comparator comparator, - Predicate hierarchy, - Predicate hierarchyInverted) { + public CsrPolyOntologyGraph(T root, + T[] nodes, + StaticCsrArray adjacencyMatrix, + Comparator comparator, + Predicate hierarchy, + Predicate hierarchyInverted) { this.root = Objects.requireNonNull(root); this.nodes = checkSorted(Objects.requireNonNull(nodes), Objects.requireNonNull(comparator)); this.adjacencyMatrix = Objects.requireNonNull(adjacencyMatrix); @@ -42,7 +45,7 @@ public CsrOntologyGraph(T root, this.hierarchyInverted = Objects.requireNonNull(hierarchyInverted); } - ImmutableCsrMatrix adjacencyMatrix() { + StaticCsrArray adjacencyMatrix() { return adjacencyMatrix; } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java similarity index 85% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilder.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java index 0ce84a036..50efc3ebb 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilder.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java @@ -1,7 +1,9 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.monarchinitiative.phenol.base.PhenolRuntimeException; import org.monarchinitiative.phenol.graph.*; +import org.monarchinitiative.phenol.graph.csr.CsrData; +import org.monarchinitiative.phenol.graph.csr.util.Util; import org.monarchinitiative.phenol.ontology.data.TermId; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -15,19 +17,19 @@ * A builder for {@link OntologyGraph} backed by a CSR matrix. * * @param data type for storing the relationships between graph nodes. - * @see CsrOntologyGraph + * @see CsrPolyOntologyGraph * @author Daniel Danis */ -public class CsrOntologyGraphBuilder implements OntologyGraphBuilder { +public class CsrPolyOntologyGraphBuilder implements OntologyGraphBuilder { - private static final Logger LOGGER = LoggerFactory.getLogger(CsrOntologyGraphBuilder.class); + private static final Logger LOGGER = LoggerFactory.getLogger(CsrPolyOntologyGraphBuilder.class); /** * Create the builder using given type for storing the edge values. */ - public static CsrOntologyGraphBuilder builder(Class clz) { + public static CsrPolyOntologyGraphBuilder builder(Class clz) { DataIndexer indexer = indexerForClass(Objects.requireNonNull(clz)); - return new CsrOntologyGraphBuilder<>(indexer); + return new CsrPolyOntologyGraphBuilder<>(indexer); } // The casts are safe since we can only use 4 supported types, and we fail for the other types. @@ -49,7 +51,7 @@ private static DataIndexer indexerForClass(Class clz) { private RelationType hierarchyRelation = RelationTypes.isA(); private final DataIndexer indexer; - private CsrOntologyGraphBuilder(DataIndexer indexer) { + private CsrPolyOntologyGraphBuilder(DataIndexer indexer) { this.indexer = Objects.requireNonNull(indexer); } @@ -59,7 +61,7 @@ private CsrOntologyGraphBuilder(DataIndexer indexer) { * {@link RelationTypes#isA()} by default. */ @Override - public CsrOntologyGraphBuilder hierarchyRelation(RelationType relationType) { + public CsrPolyOntologyGraphBuilder hierarchyRelation(RelationType relationType) { if (relationType == null) LOGGER.warn("Hierarchy relation type must not be null. Skipping.."); else @@ -68,7 +70,7 @@ public CsrOntologyGraphBuilder hierarchyRelation(RelationType relationType) { } @Override - public CsrOntologyGraph build(TermId root, Collection> edges) { + public CsrPolyOntologyGraph build(TermId root, Collection> edges) { List relationTypes = edges.stream() .map(OntologyGraphEdge::relationType) .distinct() @@ -88,12 +90,12 @@ public CsrOntologyGraph build(TermId root, Collection csrData = makeCsrData(nodes, edges, codec); - ImmutableCsrMatrix adjacencyMatrix = new ImmutableCsrMatrix<>(csrData.getIndptr(), csrData.getIndices(), csrData.getData()); + StaticCsrArray adjacencyMatrix = new StaticCsrArray<>(csrData.getIndptr(), csrData.getIndices(), csrData.getData()); LOGGER.debug("Assembling the ontology graph"); Predicate hierarchy = e -> codec.isSet(e, hierarchyRelation, false); Predicate hierarchyInverted = e -> codec.isSet(e, hierarchyRelation, true); - return new CsrOntologyGraph<>(root, nodes, adjacencyMatrix, TermId::compareTo, hierarchy, hierarchyInverted); + return new CsrPolyOntologyGraph<>(root, nodes, adjacencyMatrix, TermId::compareTo, hierarchy, hierarchyInverted); } private CsrData makeCsrData(TermId[] nodes, diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilder.java similarity index 97% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilder.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilder.java index 9b6d2f594..4dde65308 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilder.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilder.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import java.util.ArrayList; import java.util.Collections; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/DataIndexer.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexer.java similarity index 81% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/DataIndexer.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexer.java index 611dd7c70..7ca303eff 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/DataIndexer.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexer.java @@ -1,9 +1,9 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; -import org.monarchinitiative.phenol.graph.csr.indexer.DataIndexerByte; -import org.monarchinitiative.phenol.graph.csr.indexer.DataIndexerInteger; -import org.monarchinitiative.phenol.graph.csr.indexer.DataIndexerLong; -import org.monarchinitiative.phenol.graph.csr.indexer.DataIndexerShort; +import org.monarchinitiative.phenol.graph.csr.poly.indexer.DataIndexerByte; +import org.monarchinitiative.phenol.graph.csr.poly.indexer.DataIndexerInteger; +import org.monarchinitiative.phenol.graph.csr.poly.indexer.DataIndexerLong; +import org.monarchinitiative.phenol.graph.csr.poly.indexer.DataIndexerShort; import java.util.function.IntFunction; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InfallibleMappingIterator.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java similarity index 94% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InfallibleMappingIterator.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java index bf3377e46..ba53f0a7f 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InfallibleMappingIterator.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import java.util.Iterator; import java.util.function.Function; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InsufficientWidthException.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InsufficientWidthException.java similarity index 94% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InsufficientWidthException.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InsufficientWidthException.java index 170d9b49c..061b62c79 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/InsufficientWidthException.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InsufficientWidthException.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.monarchinitiative.phenol.base.PhenolRuntimeException; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/NodeIndexIterator.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/NodeIndexIterator.java similarity index 94% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/NodeIndexIterator.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/NodeIndexIterator.java index 038c7672f..5986a9cde 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/NodeIndexIterator.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/NodeIndexIterator.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import java.util.Iterator; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodec.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodec.java similarity index 93% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodec.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodec.java index 321541d87..000a56141 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodec.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodec.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.monarchinitiative.phenol.graph.RelationType; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefault.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefault.java similarity index 97% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefault.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefault.java index 779012316..bd68c7782 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefault.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefault.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.monarchinitiative.phenol.graph.RelationType; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrix.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArray.java similarity index 67% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrix.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArray.java index cf031bf54..3ded01e0b 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrix.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArray.java @@ -1,27 +1,28 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; +import org.monarchinitiative.phenol.graph.csr.util.Util; + +import java.util.*; import java.util.function.Predicate; /** - * {@linkplain ImmutableCsrMatrix} stores {@link T} entries and allows obtaining the column indices of items + * {@linkplain StaticCsrArray} stores {@link T} entries and allows obtaining the column indices of items * that match a provided predicate. * * @param entry type. * @author Daniel Danis */ -public class ImmutableCsrMatrix { +public class StaticCsrArray { + + // TODO - implement byte matrix. private final int[] indptr; private final int[] indices; private final T[] data; - public ImmutableCsrMatrix(int[] indptr, int[] indices, T[] data) { - this.indptr = checkSequenceOfNonNegativeInts(indptr); - this.indices = checkSequenceOfNonNegativeInts(indices); + public StaticCsrArray(int[] indptr, int[] indices, T[] data) { + this.indptr = Util.checkSequenceOfNonNegativeInts(indptr); + this.indices = Util.checkSequenceOfNonNegativeInts(indices); this.data = data; } @@ -55,23 +56,6 @@ public Iterator colIndicesOfVal(int row, Predicate predicate) { return new ColIterator(start, end, predicate); } - private static int[] checkSequenceOfNonNegativeInts(int[] a) { - List offenders = new ArrayList<>(); - for (int i = 0; i < a.length; i++) { - if (a[i] < 0) { - offenders.add(String.valueOf(i)); - } - } - if (!offenders.isEmpty()) { - String msg = "Expected array of non-negative integers but the following indices were negative: " + - String.join(", ", offenders); - - throw new IllegalArgumentException(msg); - } - - return a; - } - /** * NOT thread safe! */ @@ -94,7 +78,7 @@ private ColIterator(int start, @Override public boolean hasNext() { - return current >=0; + return current >= 0; } @Override diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerByte.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerByte.java similarity index 83% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerByte.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerByte.java index d1f82c13b..598d6cafb 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerByte.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerByte.java @@ -1,6 +1,6 @@ -package org.monarchinitiative.phenol.graph.csr.indexer; +package org.monarchinitiative.phenol.graph.csr.poly.indexer; -import org.monarchinitiative.phenol.graph.csr.DataIndexer; +import org.monarchinitiative.phenol.graph.csr.poly.DataIndexer; import java.util.function.IntFunction; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerInteger.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerInteger.java similarity index 83% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerInteger.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerInteger.java index 6ad0b82aa..7a3dab077 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerInteger.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerInteger.java @@ -1,6 +1,6 @@ -package org.monarchinitiative.phenol.graph.csr.indexer; +package org.monarchinitiative.phenol.graph.csr.poly.indexer; -import org.monarchinitiative.phenol.graph.csr.DataIndexer; +import org.monarchinitiative.phenol.graph.csr.poly.DataIndexer; import java.util.function.IntFunction; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerLong.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerLong.java similarity index 83% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerLong.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerLong.java index 26f3748f4..8757223bd 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerLong.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerLong.java @@ -1,6 +1,6 @@ -package org.monarchinitiative.phenol.graph.csr.indexer; +package org.monarchinitiative.phenol.graph.csr.poly.indexer; -import org.monarchinitiative.phenol.graph.csr.DataIndexer; +import org.monarchinitiative.phenol.graph.csr.poly.DataIndexer; import java.util.function.IntFunction; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerShort.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerShort.java similarity index 83% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerShort.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerShort.java index e592d39fa..aaeec1fac 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/DataIndexerShort.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/DataIndexerShort.java @@ -1,6 +1,6 @@ -package org.monarchinitiative.phenol.graph.csr.indexer; +package org.monarchinitiative.phenol.graph.csr.poly.indexer; -import org.monarchinitiative.phenol.graph.csr.DataIndexer; +import org.monarchinitiative.phenol.graph.csr.poly.DataIndexer; import java.util.function.IntFunction; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/package-info.java similarity index 87% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/package-info.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/package-info.java index 5071de949..6b38a57cb 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/indexer/package-info.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/indexer/package-info.java @@ -10,4 +10,4 @@ * * depending on the number of {@link org.monarchinitiative.phenol.graph.RelationType}s that we need to encode. */ -package org.monarchinitiative.phenol.graph.csr.indexer; +package org.monarchinitiative.phenol.graph.csr.poly.indexer; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/package-info.java new file mode 100644 index 000000000..a901d5faa --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/package-info.java @@ -0,0 +1,8 @@ +/** + * The package provides a {@link org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph} - an implementation + * of {@link org.monarchinitiative.phenol.graph.OntologyGraph} that supports storing >1 edge type. + *

+ * The {@link org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph} can be built + * using {@link org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraphBuilder}. + */ +package org.monarchinitiative.phenol.graph.csr.poly; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java new file mode 100644 index 000000000..32e7683db --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java @@ -0,0 +1,44 @@ +package org.monarchinitiative.phenol.graph.csr.util; + +import org.monarchinitiative.phenol.graph.NodeNotPresentInGraphException; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; + +public class Util { + private Util(){} + + public static int getIndexOfUsingBinarySearch(T node, T[] nodes, Comparator comparator) { + int idx = Arrays.binarySearch(nodes, node, comparator); + if (idx >= 0) + return idx; + else + throw new NodeNotPresentInGraphException(String.format("Item not found in the graph: %s", node)); + } + + public static int[] toIntArray(List indices) { + int[] a = new int[indices.size()]; + for (int i = 0; i < indices.size(); i++) + a[i] = indices.get(i); + return a; + } + + public static int[] checkSequenceOfNonNegativeInts(int[] a) { + List offenders = new ArrayList<>(); + for (int i = 0; i < a.length; i++) { + if (a[i] < 0) { + offenders.add(String.valueOf(i)); + } + } + if (!offenders.isEmpty()) { + String msg = "Expected array of non-negative integers but the following indices were negative: " + + String.join(", ", offenders); + + throw new IllegalArgumentException(msg); + } + + return a; + } +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/package-info.java new file mode 100644 index 000000000..c7fcebfff --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/package-info.java @@ -0,0 +1,4 @@ +/** + * Module-private package for utility functions and classes. + */ +package org.monarchinitiative.phenol.graph.csr.util; diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java index f008ed8cd..41b391647 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java @@ -1,8 +1,8 @@ package org.monarchinitiative.phenol.graph; import org.junit.jupiter.api.BeforeAll; -import org.monarchinitiative.phenol.graph.csr.CsrOntologyGraph; -import org.monarchinitiative.phenol.graph.csr.ImmutableCsrMatrix; +import org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph; +import org.monarchinitiative.phenol.graph.csr.poly.StaticCsrArray; import org.monarchinitiative.phenol.ontology.data.TermId; import java.util.function.Predicate; @@ -17,14 +17,14 @@ public class BaseOntologyGraphTest { private static final byte P = 0b10; // parent protected static final TermId UNKNOWN = TermId.of("HP:999"); - protected static CsrOntologyGraph GRAPH; + protected static CsrPolyOntologyGraph GRAPH; @BeforeAll public static void setUpBefore() { GRAPH = prepareSimpleGraph(); } - protected static CsrOntologyGraph prepareSimpleGraph() { + protected static CsrPolyOntologyGraph prepareSimpleGraph() { TermId root = TermId.of("HP:1"); TermId[] nodes = Stream.of( @@ -37,10 +37,10 @@ protected static CsrOntologyGraph prepareSimpleGraph() { int[] indptr = new int[] {0, 3, 5, 7, 9, 13, 14, 15, 16, 17, 20}; int[] indices = new int[] {1, 2, 9, 0, 3, 0, 3, 1, 2, 5, 6, 7, 9, 4, 4, 4, 9, 0, 4, 8}; Byte[] data = new Byte[] {C, C, P, P, C, P, C, P, P, C, C, C, P, P, P, P, P, C, C, C}; - ImmutableCsrMatrix am = new ImmutableCsrMatrix<>(indptr, indices, data); + StaticCsrArray am = new StaticCsrArray<>(indptr, indices, data); Predicate hierarchy = b -> (b & P) > 0; Predicate hierarchyInverted = b -> (b & C) > 0; - return new CsrOntologyGraph<>(root, nodes, am, TermId::compareTo, hierarchy, hierarchyInverted); + return new CsrPolyOntologyGraph<>(root, nodes, am, TermId::compareTo, hierarchy, hierarchyInverted); } } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java index 4d4a5be2c..9ef29a409 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java @@ -3,7 +3,7 @@ /** * Some test data for testing the CSR matrix logic. */ -class CsrDatasets { +public class CsrDatasets { /** * 

@@ -13,7 +13,7 @@ class CsrDatasets {
    *  [124 126 128 130]]
    *
*/ - static CsrData full() { + public static CsrData full() { return new CsrData<>( new int[]{0, 4, 8, 12, 16}, new int[]{0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3}, @@ -29,7 +29,7 @@ static CsrData full() { * [5 0 0 6]] * */ - static CsrData allEdges() { + public static CsrData allEdges() { return new CsrData<>( new int[]{0, 2, 3, 4, 6}, new int[]{0, 3, 1, 2, 0, 3}, @@ -44,7 +44,7 @@ static CsrData allEdges() { * [0 0 0]] * */ - static CsrData zeroes() { + public static CsrData zeroes() { return new CsrData<>( new int[]{0, 0, 0, 0}, new int[]{}, @@ -58,7 +58,7 @@ static CsrData zeroes() { * [3 0 4 5]] * */ - static CsrData rect() { + public static CsrData rect() { return new CsrData<>( new int[]{0, 2, 5}, new int[]{1, 3, 0, 2, 3}, diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilderTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilderTest.java similarity index 84% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilderTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilderTest.java index f9c8de042..10f2a386a 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphBuilderTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilderTest.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -15,7 +15,7 @@ import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertTrue; -public class CsrOntologyGraphBuilderTest { +public class CsrPolyOntologyGraphBuilderTest { private static final TermId ROOT = TermId.of("HP:1"); @@ -24,13 +24,13 @@ public class OneEdgeType { @Test public void build() { - CsrOntologyGraphBuilder builder = CsrOntologyGraphBuilder.builder(Byte.class); - CsrOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.hierarchyEdges()); + CsrPolyOntologyGraphBuilder builder = CsrPolyOntologyGraphBuilder.builder(Byte.class); + CsrPolyOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.hierarchyEdges()); assertThat(graph.root(), equalTo(ROOT)); // Check the adjacency matrix - ImmutableCsrMatrix adjacencyMatrix = graph.adjacencyMatrix(); + StaticCsrArray adjacencyMatrix = graph.adjacencyMatrix(); assertThat(adjacencyMatrix.indptr(), equalTo(new int[]{0, 3, 5, 7, 9, 13, 14, 15, 16, 17, 20})); assertThat(adjacencyMatrix.indices(), equalTo(new int[]{1, 2, 9, 0, 3, 0, 3, 1, 2, 5, 6, 7, 9, 4, 4, 4, 9, 0, 4, 8})); assertThat(adjacencyMatrix.data(), equalTo(new byte[]{2, 2, 1, 1, 2, 1, 2, 1, 1, 2, 2, 2, 1, 1, 1, 1, 1, 2, 2, 2})); @@ -42,13 +42,13 @@ public class MoreEdgeTypes { @Test public void build() { - CsrOntologyGraphBuilder builder = CsrOntologyGraphBuilder.builder(Byte.class); - CsrOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.makeHierarchyAndPartOfEdges()); + CsrPolyOntologyGraphBuilder builder = CsrPolyOntologyGraphBuilder.builder(Byte.class); + CsrPolyOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.makeHierarchyAndPartOfEdges()); assertThat(graph.root(), equalTo(TermId.of("HP:1"))); // Check the adjacency matrix - ImmutableCsrMatrix adjacencyMatrix = graph.adjacencyMatrix(); + StaticCsrArray adjacencyMatrix = graph.adjacencyMatrix(); assertThat(adjacencyMatrix.indptr(), equalTo(new int[]{0, 3, 5, 8, 11, 15, 16, 17, 18, 19, 23, 24, 25, 26})); assertThat(adjacencyMatrix.indices(), equalTo(new int[]{ @@ -86,7 +86,7 @@ public void build() { @Nested public class TraversalTests { - private final CsrOntologyGraph graph = buildExampleGraph(); + private final CsrPolyOntologyGraph graph = buildExampleGraph(); @ParameterizedTest @CsvSource({ @@ -151,8 +151,8 @@ public void getAncestors(TermId source, String payload) { iterableContainsTheExpectedItems(iterable, expected); } - private CsrOntologyGraph buildExampleGraph() { - return CsrOntologyGraphBuilder.builder(Byte.class) + private CsrPolyOntologyGraph buildExampleGraph() { + return CsrPolyOntologyGraphBuilder.builder(Byte.class) .hierarchyRelation(RelationTypes.isA()) .build(ROOT, OntologyGraphEdges.hierarchyEdges()); } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java similarity index 98% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java index e5d3cf301..fa246333a 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrOntologyGraphTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -16,7 +16,7 @@ import static org.hamcrest.MatcherAssert.*; import static org.hamcrest.Matchers.*; -public class CsrOntologyGraphTest extends BaseOntologyGraphTest { +public class CsrPolyOntologyGraphTest extends BaseOntologyGraphTest { @Test public void root() { diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilderTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilderTest.java similarity index 96% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilderTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilderTest.java index ab0b65fee..9a1a9db34 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrRowBuilderTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrRowBuilderTest.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/DataIndexerTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexerTest.java similarity index 98% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/DataIndexerTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexerTest.java index 16363ea76..7d4603f64 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/DataIndexerTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/DataIndexerTest.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.Test; diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefaultTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefaultTest.java similarity index 95% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefaultTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefaultTest.java index 65b53f4b6..2fad6dac9 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/RelationCodecDefaultTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/RelationCodecDefaultTest.java @@ -1,10 +1,8 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; import org.monarchinitiative.phenol.graph.RelationType; import org.monarchinitiative.phenol.graph.RelationTypes; @@ -104,6 +102,4 @@ public void isSet() { } } - - } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrixTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java similarity index 83% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrixTest.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java index d9cf01673..a79be2a0c 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/ImmutableCsrMatrixTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java @@ -1,8 +1,10 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.monarchinitiative.phenol.graph.csr.CsrData; +import org.monarchinitiative.phenol.graph.csr.CsrDatasets; import java.util.*; import java.util.stream.Collectors; @@ -12,7 +14,7 @@ import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertThrows; -public class ImmutableCsrMatrixTest { +public class StaticCsrArrayTest { @ParameterizedTest @CsvSource({ @@ -22,7 +24,7 @@ public class ImmutableCsrMatrixTest { "3, 0;1;2;3", }) public void colIndicesOfVal_full(int idx, String payload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.full()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.full()); List expected = decodeExpectedPayload(payload); List actual = intoList(matrix.colIndicesOfVal(idx, k -> true)); assertThat(actual, equalTo(expected)); @@ -36,7 +38,7 @@ public void colIndicesOfVal_full(int idx, String payload) { "3, 0;3", }) public void colIndicesOfVal_allEdges(int idx, String payload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.allEdges()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.allEdges()); List expected = decodeExpectedPayload(payload); List actual = intoList(matrix.colIndicesOfVal(idx, k -> true)); assertThat(actual, equalTo(expected)); @@ -50,7 +52,7 @@ public void colIndicesOfVal_allEdges(int idx, String payload) { "3, 3, 0", }) public void colIndicesOfVal_allEdges_evenAndOddValues(int idx, String evenPayload, String oddPayload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.allEdges()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.allEdges()); List evenExpected = decodeExpectedPayload(evenPayload); List evenActual = intoList(matrix.colIndicesOfVal(idx, k -> k % 2 == 0)); @@ -68,7 +70,7 @@ public void colIndicesOfVal_allEdges_evenAndOddValues(int idx, String evenPayloa "2, ''", }) public void colIndicesOfVal_zeroes(int idx, String payload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.zeroes()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.zeroes()); List expected = decodeExpectedPayload(payload); List actual = intoList(matrix.colIndicesOfVal(idx, k -> true)); assertThat(actual, equalTo(expected)); @@ -80,7 +82,7 @@ public void colIndicesOfVal_zeroes(int idx, String payload) { "1, '0;2;3'", }) public void colIndicesOfVal_rect(int idx, String payload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.rect()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.rect()); List expected = decodeExpectedPayload(payload); List actual = intoList(matrix.colIndicesOfVal(idx, k -> true)); assertThat(actual, equalTo(expected)); @@ -92,7 +94,7 @@ public void colIndicesOfVal_rect(int idx, String payload) { "1, 2, 0;3" }) public void colIndicesOfVal_rect_evenAndOddValues(int idx, String evenPayload, String oddPayload) { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.rect()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.rect()); List evenExpected = decodeExpectedPayload(evenPayload); List evenActual = intoList(matrix.colIndicesOfVal(idx, k -> k % 2 == 0)); @@ -105,7 +107,7 @@ public void colIndicesOfVal_rect_evenAndOddValues(int idx, String evenPayload, S @Test public void colIndicesOfVal_rect_unfound() { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.rect()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.rect()); // Here we're searching for `3` in the 0th row, but it is not there. assertThat(intoList(matrix.colIndicesOfVal(0, k -> k == 3)), @@ -118,7 +120,7 @@ public void colIndicesOfVal_rect_unfound() { @Test public void accessingNegativeOrBeyondBoundsIndexThrows() { - ImmutableCsrMatrix matrix = makeCsrMatrix(CsrDatasets.allEdges()); + StaticCsrArray matrix = makeCsrMatrix(CsrDatasets.allEdges()); IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> matrix.colIndicesOfVal(-1, entry -> true)); @@ -135,7 +137,7 @@ public void invalidIndptrInputDataThrow() { int[] indptr = {0, 2, -1}; int[] indices = {1, 3, 0, 2, 3}; Integer[] data = {1, 2, 3, 4, 5}; - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> new ImmutableCsrMatrix<>(indptr, indices, data)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> new StaticCsrArray<>(indptr, indices, data)); assertThat(e.getMessage(), equalTo("Expected array of non-negative integers but the following indices were negative: 2")); } @@ -146,7 +148,7 @@ public void invalidIndicesInputDataThrow() { int[] indptr = {0, 2, 5}; int[] indices = {-1, 3, 0, 2, 3}; Integer[] data = {1, 2, 3, 4, 5}; - IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> new ImmutableCsrMatrix<>(indptr, indices, data)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> new StaticCsrArray<>(indptr, indices, data)); assertThat(e.getMessage(), equalTo("Expected array of non-negative integers but the following indices were negative: 0")); } @@ -167,7 +169,7 @@ private static List decodeExpectedPayload(String payload) { } - private static ImmutableCsrMatrix makeCsrMatrix(CsrData data) { - return new ImmutableCsrMatrix<>(data.getIndptr(), data.getIndices(), data.getData()); + private static StaticCsrArray makeCsrMatrix(CsrData data) { + return new StaticCsrArray<>(data.getIndptr(), data.getIndices(), data.getData()); } } From 312ad5a38d48a8d1400cf1026fc084239a57f3d3 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 20:31:52 -0400 Subject: [PATCH 3/9] Implement `CsrMonoOntologyGraph`. Fix bug in `CsrPolyOntologyGraph` that supports to iterate over the iterable only once. Improve `OntologyGraph` testing. --- .../phenol/graph/csr/mono/CsrData.java | 26 + .../graph/csr/mono/CsrMonoOntologyGraph.java | 100 ++++ .../csr/mono/CsrMonoOntologyGraphBuilder.java | 101 ++++ .../graph/csr/mono/SetIncludingSource.java | 67 +++ .../phenol/graph/csr/mono/StaticCsrArray.java | 91 +++ .../graph/csr/mono/TraversingIterator.java | 43 ++ .../phenol/graph/csr/mono/package-info.java | 8 + .../phenol/graph/csr/{ => poly}/CsrData.java | 2 +- .../graph/csr/poly/CsrPolyOntologyGraph.java | 35 +- .../csr/poly/CsrPolyOntologyGraphBuilder.java | 36 +- .../csr/poly/InfallibleMappingIterator.java | 5 +- .../phenol/graph/csr/util/Util.java | 40 +- .../phenol/graph/BaseOntologyGraphTest.java | 46 -- .../phenol/graph/OntologyGraphTest.java | 519 +++++++++++++----- .../mono/CsrMonoOntologyGraphBuilderTest.java | 91 +++ .../csr/mono/CsrMonoOntologyGraphTest.java | 19 + .../graph/csr/{ => poly}/CsrDatasets.java | 12 +- .../csr/poly/CsrPolyOntologyGraphTest.java | 265 ++------- .../graph/csr/poly/StaticCsrArrayTest.java | 2 - 19 files changed, 1040 insertions(+), 468 deletions(-) create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrData.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/TraversingIterator.java create mode 100644 phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java rename phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/CsrData.java (91%) delete mode 100644 phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java create mode 100644 phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java create mode 100644 phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphTest.java rename phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/{ => poly}/CsrDatasets.java (81%) diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrData.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrData.java new file mode 100644 index 000000000..77b0ec768 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrData.java @@ -0,0 +1,26 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +/** + * Essentially a record with {@link StaticCsrArray}s for getting parents and children. + * + * @param type of data. + * @author Daniel Danis + */ +class CsrData { + + private final StaticCsrArray parents; + private final StaticCsrArray children; + + CsrData(StaticCsrArray parents, StaticCsrArray children) { + this.parents = parents; + this.children = children; + } + + StaticCsrArray getParents() { + return parents; + } + + StaticCsrArray getChildren() { + return children; + } +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java new file mode 100644 index 000000000..e2b06df79 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java @@ -0,0 +1,100 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import org.monarchinitiative.phenol.graph.OntologyGraph; +import org.monarchinitiative.phenol.graph.csr.util.Util; +import org.monarchinitiative.phenol.utils.IterableIteratorWrapper; + +import java.util.*; + +/** + * An {@link OntologyGraph} that only supports one edge type and supports efficient retrieval of parent or child nodes. + * + * @param type of the term/graph node. + * @author Daniel Danis + */ +public class CsrMonoOntologyGraph implements OntologyGraph { + + private final T root; + private final T[] nodes; + private final Comparator comparator; + private final StaticCsrArray parents; + private final StaticCsrArray children; + + CsrMonoOntologyGraph(T root, + T[] nodes, + Comparator comparator, + StaticCsrArray parents, + StaticCsrArray children) { + this.root = Objects.requireNonNull(root); + this.nodes = Objects.requireNonNull(nodes); + this.comparator = Objects.requireNonNull(comparator); + this.parents = Objects.requireNonNull(parents); + this.children = Objects.requireNonNull(children); + } + + StaticCsrArray getParentArray() { + return parents; + } + + StaticCsrArray getChildArray() { + return children; + } + + T[] getNodes() { + return nodes; + } + + @Override + public T root() { + return root; + } + + @Override + public Iterable getChildren(T source, boolean includeSource) { + return getImmediateNeighbors(children, source, includeSource); + } + + @Override + public Iterable getDescendants(T source, boolean includeSource) { + // Check if `source` is in the graph. + int intentionallyUnused = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + + return new IterableIteratorWrapper<>(() -> new TraversingIterator<>(source, src -> getChildren(src, includeSource))); + } + + @Override + public Iterable getParents(T source, boolean includeSource) { + return getImmediateNeighbors(parents, source, includeSource); + } + + @Override + public Iterable getAncestors(T source, boolean includeSource) { + // Check if `source` is in the graph. + int intentionallyUnused = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + + return new IterableIteratorWrapper<>(() -> new TraversingIterator<>(source, src -> getParents(src, includeSource))); + } + + private Iterable getImmediateNeighbors(StaticCsrArray array, + T source, + boolean includeSource) { + int index = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + Set nodes = array.getOutgoingNodes(index); + + return includeSource + ? new SetIncludingSource<>(source, nodes) + : nodes; + } + + @Override + public int size() { + return nodes.length; + } + + @Override + public Iterator iterator() { + // TODO - can we do better here? + return Arrays.stream(nodes).iterator(); + } + +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java new file mode 100644 index 000000000..0ea550606 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java @@ -0,0 +1,101 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import org.monarchinitiative.phenol.graph.*; +import org.monarchinitiative.phenol.graph.csr.util.Util; +import org.monarchinitiative.phenol.ontology.data.TermId; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Builder for {@link CsrMonoOntologyGraphBuilder}. + * + * @author Daniel Danis + */ +public class CsrMonoOntologyGraphBuilder implements OntologyGraphBuilder { + + private static final Logger LOGGER = LoggerFactory.getLogger(CsrMonoOntologyGraphBuilder.class); + + private RelationType hierarchyRelation = RelationTypes.isA(); + + /** + * Create the builder. + */ + public static CsrMonoOntologyGraphBuilder builder() { + return new CsrMonoOntologyGraphBuilder(); + } + + @Override + public OntologyGraphBuilder hierarchyRelation(RelationType relationType) { + if (relationType == null) + LOGGER.warn("Hierarchy relation type must not be null. Skipping.."); + else + this.hierarchyRelation = relationType; + return this; + } + + @Override + public CsrMonoOntologyGraph build(TermId root, Collection> edges) { + LOGGER.debug("Extracting edges with target hierarchy relation {}", hierarchyRelation.label()); + List> hierarchyEdges = edges.stream() + .filter(e -> e.relationType().equals(hierarchyRelation)) + .collect(Collectors.toList()); + + LOGGER.debug("Sorting graph nodes"); + TermId[] nodes = edges.stream() + .flatMap(e -> Stream.of(e.subject(), e.object())) + .distinct() + .sorted(TermId::compareTo) + .toArray(TermId[]::new); + + CsrData csrData = makeCsrData(nodes, hierarchyEdges); + + return new CsrMonoOntologyGraph<>(root, nodes, TermId::compareTo, csrData.getParents(), csrData.getChildren()); + } + + private CsrData makeCsrData(TermId[] nodes, + Collection> edges) { + Map>> adjacentEdges = Util.findAdjacentEdges(nodes, edges); + + List parentIndptr = new ArrayList<>(); + parentIndptr.add(0); + List parents = new ArrayList<>(); + + List childIndptr = new ArrayList<>(); + childIndptr.add(0); + List children = new ArrayList<>(); + + for (int rowIdx = 0; rowIdx < nodes.length; rowIdx++) { + TermId source = nodes[rowIdx]; + List> adjacent = adjacentEdges.getOrDefault(rowIdx, List.of()); + + for (OntologyGraphEdge edge : adjacent) { + // `inverted == true` if `src == subject` (child) and `object` is parent. + boolean targetIsChild = source.equals(edge.object()); + TermId target = targetIsChild ? edge.subject() : edge.object(); + if (targetIsChild) { + // edge where `subject` is child and `object` is parent. + children.add(target); + } else { + // edge where `subject` is parent and `object` is child. + parents.add(target); + } + } + + parentIndptr.add(parents.size()); + childIndptr.add(children.size()); + } + + StaticCsrArray parentsArray = new StaticCsrArray<>(Util.toIntArray(parentIndptr), parents.toArray(new TermId[0])); + StaticCsrArray childrenArray = new StaticCsrArray<>(Util.toIntArray(childIndptr), children.toArray(new TermId[0])); + + return new CsrData<>(parentsArray, childrenArray); + } + +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java new file mode 100644 index 000000000..66ed3fd12 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java @@ -0,0 +1,67 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import java.util.AbstractSet; +import java.util.Iterator; +import java.util.Set; + +/** + * A {@link Set} that contains an item and all elements of the other set. + * + * @param – the type of elements in this set + * @author Daniel Danis + */ +class SetIncludingSource extends AbstractSet { + + private final T item; + private final Set other; + + SetIncludingSource(T item, Set other) { + this.item = item; + this.other = other; + } + + @Override + public Iterator iterator() { + return new IncludingIterator<>(item, other.iterator()); + } + + @Override + public int size() { + return other.size() + 1; + } + + /** + * An {@link Iterator} that first yields the first item and then the items from the remaining iterator. + *

+ * NOT THREAD SAFE, of course! + * + * @param – the type of elements in this iterator + */ + private static class IncludingIterator implements Iterator { + + private final T first; + private final Iterator remaining; + private boolean yieldedFirst = false; + + private IncludingIterator(T first, Iterator remaining) { + this.first = first; + this.remaining = remaining; + } + + @Override + public boolean hasNext() { + return !yieldedFirst || remaining.hasNext(); + } + + @Override + public T next() { + if (!yieldedFirst) { + yieldedFirst = true; + return first; + } + + return remaining.next(); + } + } + +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java new file mode 100644 index 000000000..0ee078371 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java @@ -0,0 +1,91 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import org.monarchinitiative.phenol.graph.csr.util.Util; + +import java.util.*; + +/** + * A simple array for retrieval of adjacent graph nodes. + *

+ * The array is inspired by CSR format, but it does not need the column indices and stores directly + * the {@link T} elements that represent the adjacent nodes. Since we are mostly interested in propagating relationships + * such as is_a, the adjacent corresponds either to parents or children most of the time. + * + * @param – the type of elements in this array is generic over (most likely term ID). + */ +class StaticCsrArray { + + private final int[] indptr; + private final T[] data; + + StaticCsrArray(int[] indptr, T[] data) { + this.indptr = Util.checkSequenceOfNonNegativeInts(indptr); + this.data = data; + } + + int[] getIndptr() { + return indptr; + } + + T[] getData() { + return data; + } + + Set getOutgoingNodes(int row) { + if (row < 0 || indptr.length < row) + throw new NoSuchElementException(); + + int start = indptr[row]; + int end = indptr[row + 1]; + + if (start == end) + return Collections.emptySet(); + + return new AbstractSet<>() { + @Override + public Iterator iterator() { + return new CsrArrayIterator<>(data, start, end); + } + + @Override + public int size() { + return end - start; + } + }; + } + + /** + * An iterator over items associated with + * NOT thread safe! + */ + private static class CsrArrayIterator implements Iterator { + + private final T[] data; + private int cursor; + private final int end; + + private CsrArrayIterator(T[] data, + int start, + int end) { + this.data = data; + assert start <= end; + this.cursor = start; + this.end = end; + } + + @Override + public boolean hasNext() { + return cursor < end; + } + + @Override + public T next() { + try { + return data[cursor++]; + } catch (IndexOutOfBoundsException e) { + throw new NoSuchElementException(); + } + } + + } +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/TraversingIterator.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/TraversingIterator.java new file mode 100644 index 000000000..7c42f2671 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/TraversingIterator.java @@ -0,0 +1,43 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import java.util.*; +import java.util.function.Function; + +/** + * An iterator that implements breadth-first search over starting from a source node. + * + * @param – the type of elements in producing {@link org.monarchinitiative.phenol.graph.OntologyGraph} + * is generic over (most likely term ID). + */ +class TraversingIterator implements Iterator { + + private final Function> neighbors; + private final Set seen = new HashSet<>(); + private final Deque buffer = new ArrayDeque<>(); + + TraversingIterator(T source, + Function> neighbors) { + this.neighbors = neighbors; + for (T item : neighbors.apply(source)) { + seen.add(item); + buffer.add(item); + } + } + + @Override + public boolean hasNext() { + return !buffer.isEmpty(); + } + + @Override + public T next() { + T current = buffer.pop(); + for (T item : neighbors.apply(current)) { + if (!seen.contains(item)) { + seen.add(item); + buffer.add(item); + } + } + return current; + } +} diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java new file mode 100644 index 000000000..b15102061 --- /dev/null +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java @@ -0,0 +1,8 @@ +/** + * {@linkplain org.monarchinitiative.phenol.graph.csr.mono} package provides a CSR implementation + * of {@link org.monarchinitiative.phenol.graph.OntologyGraph} backed by a single edge type. + * + * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraph + * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraphBuilder + */ +package org.monarchinitiative.phenol.graph.csr.mono; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrData.java similarity index 91% rename from phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java rename to phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrData.java index 113158f2d..c4e6af306 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/CsrData.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrData.java @@ -1,4 +1,4 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; /** * A POJO for the data structures that back a CSR matrix. diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java index a0736c3cb..11b8523ba 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraph.java @@ -7,6 +7,7 @@ import java.util.*; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Stream; /** @@ -62,7 +63,8 @@ public T root() { */ @Override public Iterable getChildren(T source, boolean includeSource) { - Iterator base = getNodeIndicesWithRelationship(source, hierarchyInverted, includeSource); + int idx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + Supplier> base = () -> getColsWithRelationship(idx, hierarchyInverted, includeSource); return new IterableIteratorWrapper<>(() -> new InfallibleMappingIterator<>(base, this::getNodeForIndex)); } @@ -71,7 +73,8 @@ public Iterable getChildren(T source, boolean includeSource) { */ @Override public Iterable getDescendants(T source, boolean includeSource) { - Iterator base = traverseGraph(source, hierarchyInverted, includeSource); + int idx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + Supplier> base = () -> traverseGraph(idx, hierarchyInverted, includeSource); return new IterableIteratorWrapper<>(() -> new InfallibleMappingIterator<>(base, this::getNodeForIndex)); } @@ -80,7 +83,8 @@ public Iterable getDescendants(T source, boolean includeSource) { */ @Override public Iterable getParents(T source, boolean includeSource) { - Iterator base = getNodeIndicesWithRelationship(source, hierarchy, includeSource); + int idx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + Supplier> base = () -> getColsWithRelationship(idx, hierarchy, includeSource); return new IterableIteratorWrapper<>(() -> new InfallibleMappingIterator<>(base, this::getNodeForIndex)); } @@ -89,7 +93,8 @@ public Iterable getParents(T source, boolean includeSource) { */ @Override public Iterable getAncestors(T source, boolean includeSource) { - Iterator base = traverseGraph(source, hierarchy, includeSource); + int idx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + Supplier> base = () -> traverseGraph(idx, hierarchy, includeSource); return new IterableIteratorWrapper<>(() -> new InfallibleMappingIterator<>(base, this::getNodeForIndex)); } @@ -98,7 +103,8 @@ public Iterable getAncestors(T source, boolean includeSource) { */ @Override public boolean isLeaf(T source) { - return !getNodeIndicesWithRelationship(source, hierarchyInverted, false).hasNext(); + int idx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + return !getNodeIndicesWithRelationship(idx, hierarchyInverted).hasNext(); } /** @@ -134,11 +140,8 @@ private static T[] checkSorted(T[] a, Comparator comparator) { return a; } - private Iterator getNodeIndicesWithRelationship(T source, - Predicate relationship, - boolean includeSource) { - int rowIdx = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); - return getColsWithRelationship(rowIdx, relationship, includeSource); + private Iterator getNodeIndicesWithRelationship(int sourceIdx, Predicate relationship) { + return getColsWithRelationship(sourceIdx, relationship, false); } private Iterator getColsWithRelationship(int source, @@ -152,10 +155,10 @@ private T getNodeForIndex(int idx) { return nodes[idx]; } - private Iterator traverseGraph(T source, + private Iterator traverseGraph(int sourceIdx, Predicate relationship, boolean includeSource) { - return new TraversingIterator(source, relationship, includeSource); + return new TraversingIterator(sourceIdx, relationship, includeSource); } /** @@ -167,11 +170,11 @@ private class TraversingIterator implements Iterator { private final Deque buffer = new ArrayDeque<>(); private final Predicate relationship; - private TraversingIterator(T source, - Predicate relationship, - boolean includeSource) { + private TraversingIterator(int sourceIdx, + Predicate relationship, + boolean includeSource) { this.relationship = relationship; - Iterator base = getNodeIndicesWithRelationship(source, relationship, includeSource); + Iterator base = getColsWithRelationship(sourceIdx, relationship, includeSource); while (base.hasNext()) { int idx = base.next(); seen.add(idx); diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java index 50efc3ebb..ca573c694 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphBuilder.java @@ -2,7 +2,6 @@ import org.monarchinitiative.phenol.base.PhenolRuntimeException; import org.monarchinitiative.phenol.graph.*; -import org.monarchinitiative.phenol.graph.csr.CsrData; import org.monarchinitiative.phenol.graph.csr.util.Util; import org.monarchinitiative.phenol.ontology.data.TermId; import org.slf4j.Logger; @@ -106,7 +105,7 @@ private CsrData makeCsrData(TermId[] nodes, List indices = new ArrayList<>(); List data = new ArrayList<>(); - Map>> adjacentEdges = findAdjacentEdges(nodes, edges); + Map>> adjacentEdges = Util.findAdjacentEdges(nodes, edges); CsrRowBuilder row = new CsrRowBuilder<>(indexer); for (int rowIdx = 0; rowIdx < nodes.length; rowIdx++) { @@ -132,37 +131,4 @@ private CsrData makeCsrData(TermId[] nodes, return new CsrData<>(Util.toIntArray(indptr), Util.toIntArray(indices), data.toArray(indexer.createArray())); } - /** - * We build the CSR matrix row by row, and we need to know about all nodes that are adjacent with - * (have a relationship with) the node represented by the row under the construction. - * Here we prepare a mapping from the row index to a list of all adjacent edges. - */ - private static Map>> findAdjacentEdges(TermId[] nodes, - Collection> edges) { - Map>> data = new HashMap<>(); - - TermId lastSub = null; - int lastSubIdx = -1; - - for (OntologyGraphEdge edge : edges) { - int subIdx; - TermId sub = edge.subject(); - if (sub == lastSub) { - subIdx = lastSubIdx; - } else { - lastSub = sub; - subIdx = Util.getIndexOfUsingBinarySearch(sub, nodes, TermId::compareTo); - lastSubIdx = subIdx; - } - - TermId obj = edge.object(); - int objIdx = Util.getIndexOfUsingBinarySearch(obj, nodes, TermId::compareTo); - - data.computeIfAbsent(subIdx, x -> new ArrayList<>()).add(edge); - data.computeIfAbsent(objIdx, x -> new ArrayList<>()).add(edge); - } - - return data; - } - } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java index ba53f0a7f..704d6f2a6 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/poly/InfallibleMappingIterator.java @@ -2,6 +2,7 @@ import java.util.Iterator; import java.util.function.Function; +import java.util.function.Supplier; /** * {@linkplain InfallibleMappingIterator} applies a {@code mapper} function to each item from the {@code base} iterator. @@ -16,8 +17,8 @@ class InfallibleMappingIterator implements Iterator { private final Iterator base; private final Function mapper; - InfallibleMappingIterator(Iterator base, Function mapper) { - this.base = base; + InfallibleMappingIterator(Supplier> base, Function mapper) { + this.base = base.get(); this.mapper = mapper; } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java index 32e7683db..cfcc31e75 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/util/Util.java @@ -1,11 +1,10 @@ package org.monarchinitiative.phenol.graph.csr.util; import org.monarchinitiative.phenol.graph.NodeNotPresentInGraphException; +import org.monarchinitiative.phenol.graph.OntologyGraphEdge; +import org.monarchinitiative.phenol.ontology.data.TermId; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Comparator; -import java.util.List; +import java.util.*; public class Util { private Util(){} @@ -41,4 +40,37 @@ public static int[] checkSequenceOfNonNegativeInts(int[] a) { return a; } + + /** + * We build the CSR matrix row by row, and we need to know about all nodes that are adjacent with + * (have a relationship with) the node represented by the row under the construction. + * Here we prepare a mapping from the row index to a list of all adjacent edges. + */ + public static Map>> findAdjacentEdges(TermId[] nodes, + Collection> edges) { + Map>> data = new HashMap<>(); + + TermId lastSub = null; + int lastSubIdx = -1; + + for (OntologyGraphEdge edge : edges) { + int subIdx; + TermId sub = edge.subject(); + if (sub == lastSub) { + subIdx = lastSubIdx; + } else { + lastSub = sub; + subIdx = getIndexOfUsingBinarySearch(sub, nodes, TermId::compareTo); + lastSubIdx = subIdx; + } + + TermId obj = edge.object(); + int objIdx = getIndexOfUsingBinarySearch(obj, nodes, TermId::compareTo); + + data.computeIfAbsent(subIdx, x -> new ArrayList<>()).add(edge); + data.computeIfAbsent(objIdx, x -> new ArrayList<>()).add(edge); + } + + return data; + } } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java deleted file mode 100644 index 41b391647..000000000 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/BaseOntologyGraphTest.java +++ /dev/null @@ -1,46 +0,0 @@ -package org.monarchinitiative.phenol.graph; - -import org.junit.jupiter.api.BeforeAll; -import org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph; -import org.monarchinitiative.phenol.graph.csr.poly.StaticCsrArray; -import org.monarchinitiative.phenol.ontology.data.TermId; - -import java.util.function.Predicate; -import java.util.stream.Stream; - -/** - * Base for {@link org.monarchinitiative.phenol.graph.OntologyGraph} tests that provides small CSR graph for testing. - */ -public class BaseOntologyGraphTest { - - private static final byte C = 0b01; // child - private static final byte P = 0b10; // parent - - protected static final TermId UNKNOWN = TermId.of("HP:999"); - protected static CsrPolyOntologyGraph GRAPH; - - @BeforeAll - public static void setUpBefore() { - GRAPH = prepareSimpleGraph(); - } - - protected static CsrPolyOntologyGraph prepareSimpleGraph() { - TermId root = TermId.of("HP:1"); - - TermId[] nodes = Stream.of( - "HP:01", "HP:010", "HP:011", "HP:0110", - "HP:02", "HP:020", "HP:021", "HP:022", - "HP:03", "HP:1") - .map(TermId::of) - .toArray(TermId[]::new); - - int[] indptr = new int[] {0, 3, 5, 7, 9, 13, 14, 15, 16, 17, 20}; - int[] indices = new int[] {1, 2, 9, 0, 3, 0, 3, 1, 2, 5, 6, 7, 9, 4, 4, 4, 9, 0, 4, 8}; - Byte[] data = new Byte[] {C, C, P, P, C, P, C, P, P, C, C, C, P, P, P, P, P, C, C, C}; - StaticCsrArray am = new StaticCsrArray<>(indptr, indices, data); - Predicate hierarchy = b -> (b & P) > 0; - Predicate hierarchyInverted = b -> (b & C) > 0; - - return new CsrPolyOntologyGraph<>(root, nodes, am, TermId::compareTo, hierarchy, hierarchyInverted); - } -} diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/OntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/OntologyGraphTest.java index f6c1127af..c47f9ff6f 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/OntologyGraphTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/OntologyGraphTest.java @@ -1,164 +1,435 @@ package org.monarchinitiative.phenol.graph; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.monarchinitiative.phenol.ontology.data.TermId; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import static org.junit.jupiter.api.Assertions.*; import static org.hamcrest.MatcherAssert.*; import static org.hamcrest.Matchers.*; -public class OntologyGraphTest extends BaseOntologyGraphTest { - - @ParameterizedTest - @CsvSource({ - // True examples - "HP:01, HP:1, true", +/** + * The test suite that checks that an implementation of {@link OntologyGraph} meets the specification. + */ +public abstract class OntologyGraphTest { - "HP:010, HP:01, true", - "HP:011, HP:01, true", - "HP:0110, HP:011, true", + private static final TermId UNKNOWN = TermId.of("HP:999"); - "HP:02, HP:1, true", - "HP:020, HP:02, true", - "HP:021, HP:02, true", - "HP:022, HP:02, true", + private OntologyGraph graph; - "HP:03, HP:1, true", + /** + * Get an ontology graph that should be tested. + */ + protected abstract OntologyGraph getGraph(); - // False examples - "HP:1, HP:1, false", - "HP:01, HP:02, false", - "HP:020, HP:01, false", - "HP:03, HP:0110, false", - }) - public void isChildOf(TermId subject, TermId object, boolean expected) { - assertThat(GRAPH.isChildOf(subject, object), equalTo(expected)); + @BeforeEach + public void setUp() { + graph = getGraph(); } @Test - public void isChildOfUnknownSource() { - TermId ok = TermId.of("HP:01"); - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.isChildOf(ok, UNKNOWN)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - - assertThat(GRAPH.isChildOf(UNKNOWN, ok), equalTo(false)); + public void root() { + TermId root = graph.root(); + assertThat(root.getValue(), equalTo("HP:1")); } - @ParameterizedTest - @CsvSource({ - // True examples - "HP:010, HP:1, true", - "HP:011, HP:1, true", - "HP:0110, HP:1, true", - "HP:0110, HP:01, true", - - "HP:020, HP:1, true", - "HP:021, HP:1, true", - "HP:022, HP:1, true", - - // False examples - "HP:1, HP:1, false", - "HP:1, HP:01, false", - }) - public void isDescendantOf(TermId subject, TermId object, boolean expected) { - assertThat(GRAPH.isDescendantOf(subject, object), equalTo(expected)); + @Nested + public class ChildrenTraversal { + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:01;HP:02;HP:03", + "HP:02, HP:020;HP:021;HP:022", + "HP:03, ''", + }) + public void getChildren(TermId source, String payload) { + Iterable iterable = graph.getChildren(source, false); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:1;HP:01;HP:02;HP:03", + "HP:02, HP:02;HP:020;HP:021;HP:022", + "HP:03, HP:03", + }) + public void getChildrenIncludingTheSource(TermId source, String payload) { + Iterable iterable = graph.getChildren(source, true); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @Test + public void getChildrenUnknownSource() { + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.getChildren(UNKNOWN, false)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + } + + @ParameterizedTest + @CsvSource({ + // True examples + "HP:01, HP:1, true", + + "HP:010, HP:01, true", + "HP:011, HP:01, true", + "HP:0110, HP:011, true", + + "HP:02, HP:1, true", + "HP:020, HP:02, true", + "HP:021, HP:02, true", + "HP:022, HP:02, true", + + "HP:03, HP:1, true", + + // False examples + "HP:1, HP:1, false", + "HP:01, HP:02, false", + "HP:020, HP:01, false", + "HP:03, HP:0110, false", + }) + public void isChildOf(TermId subject, TermId object, boolean expected) { + assertThat(graph.isChildOf(subject, object), equalTo(expected)); + } + + @Test + public void isChildOfUnknownSource() { + TermId ok = TermId.of("HP:01"); + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.isChildOf(ok, UNKNOWN)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + + assertThat(graph.isChildOf(UNKNOWN, ok), equalTo(false)); + } } + @Nested + public class DescendantTraversal { + + @ParameterizedTest + @CsvSource({ + // True examples + "HP:010, HP:1, true", + "HP:011, HP:1, true", + "HP:0110, HP:1, true", + "HP:0110, HP:01, true", + + "HP:020, HP:1, true", + "HP:021, HP:1, true", + "HP:022, HP:1, true", + + // False examples + "HP:1, HP:1, false", + "HP:1, HP:01, false", + }) + public void isDescendantOf(TermId subject, TermId object, boolean expected) { + assertThat(graph.isDescendantOf(subject, object), equalTo(expected)); + } + + @Test + public void isDescendantOfUnknownSource() { + TermId ok = TermId.of("HP:01"); + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.isDescendantOf(ok, UNKNOWN)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + + assertThat(graph.isDescendantOf(UNKNOWN, ok), equalTo(false)); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:01;HP:010;HP:011;HP:0110; HP:02;HP:020;HP:021;HP:022; HP:03", + "HP:01, HP:010;HP:011;HP:0110", + "HP:010, HP:0110", + "HP:011, HP:0110", + "HP:0110, ''", + + "HP:02, HP:020;HP:021;HP:022", + "HP:020, ''", + "HP:021, ''", + "HP:022, ''", + "HP:03, ''", + }) + public void getDescendants(TermId source, String payload) { + Iterable iterable = graph.getDescendants(source, false); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:1;HP:01;HP:010;HP:011;HP:0110; HP:02;HP:020;HP:021;HP:022; HP:03", + "HP:01, HP:01;HP:010;HP:011;HP:0110", + "HP:010, HP:010;HP:0110", + "HP:011, HP:011;HP:0110", + "HP:0110, HP:0110", + + "HP:02, HP:02;HP:020;HP:021;HP:022", + "HP:020, HP:020", + "HP:021, HP:021", + "HP:022, HP:022", + "HP:03, HP:03", + }) + public void getDescendantsIncludingTheSource(TermId source, String payload) { + Iterable iterable = graph.getDescendants(source, true); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @Test + public void getDescendantsUnknownSource() { + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.getDescendants(UNKNOWN, false)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + } - @Test - public void isDescendantOfUnknownSource() { - TermId ok = TermId.of("HP:01"); - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.isDescendantOf(ok, UNKNOWN)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - - assertThat(GRAPH.isDescendantOf(UNKNOWN, ok), equalTo(false)); } - @ParameterizedTest - @CsvSource({ - // True examples - "HP:1, HP:01, true", - - "HP:01, HP:010, true", - "HP:01, HP:011, true", - "HP:011, HP:0110, true", - - "HP:1, HP:02, true", - "HP:02, HP:020, true", - "HP:02, HP:021, true", - "HP:02, HP:022, true", - - "HP:1, HP:03, true", - - // False examples - "HP:1, HP:1, false", - "HP:02, HP:01, false", - "HP:01, HP:020, false", - "HP:0110, HP:03, false", - }) - public void isParentOf(TermId subject, TermId object, boolean expected) { - assertThat(GRAPH.isParentOf(subject, object), equalTo(expected)); + @Nested + public class ParentsTraversal { + + @ParameterizedTest + @CsvSource({ + "HP:1, ''", + "HP:01, HP:1", + "HP:03, HP:1", + "HP:0110, HP:010;HP:011", + }) + public void getParents(TermId source, String payload) { + Iterable iterable = graph.getParents(source, false); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:1", + "HP:01, HP:01;HP:1", + "HP:03, HP:03;HP:1", + "HP:0110, HP:0110;HP:010;HP:011", + }) + public void getParentsIncludingTheSource(TermId source, String payload) { + Iterable iterable = graph.getParents(source, true); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @Test + public void getParentsUnknownSource() { + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.getParents(UNKNOWN, false)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + } + + @ParameterizedTest + @CsvSource({ + // True examples + "HP:1, HP:01, true", + + "HP:01, HP:010, true", + "HP:01, HP:011, true", + "HP:011, HP:0110, true", + + "HP:1, HP:02, true", + "HP:02, HP:020, true", + "HP:02, HP:021, true", + "HP:02, HP:022, true", + + "HP:1, HP:03, true", + + // False examples + "HP:1, HP:1, false", + "HP:02, HP:01, false", + "HP:01, HP:020, false", + "HP:0110, HP:03, false", + }) + public void isParentOf(TermId subject, TermId object, boolean expected) { + assertThat(graph.isParentOf(subject, object), equalTo(expected)); + } + + @Test + public void isParentOfUnknownSource() { + TermId ok = TermId.of("HP:01"); + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.isParentOf(ok, UNKNOWN)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + + assertThat(graph.isParentOf(UNKNOWN, ok), equalTo(false)); + } + } - @Test - public void isParentOfUnknownSource() { - TermId ok = TermId.of("HP:01"); - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.isParentOf(ok, UNKNOWN)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + @Nested + public class AncestorTraversal { + + @ParameterizedTest + @CsvSource({ + "HP:1, ''", + "HP:01, HP:1", + "HP:0110, HP:010;HP:011;HP:01;HP:1", + "HP:022, HP:02;HP:1", + "HP:03, HP:1", + }) + public void getAncestors(TermId source, String payload) { + Iterable iterable = graph.getAncestors(source, false); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, HP:1", + "HP:01, HP:01;HP:1", + "HP:0110, HP:0110;HP:010;HP:011;HP:01;HP:1", + "HP:022, HP:022;HP:02;HP:1", + "HP:03, HP:03;HP:1", + }) + public void getAncestorsIncludingTheSource(TermId source, String payload) { + Iterable iterable = graph.getAncestors(source, true); + Set expected = parsePayload(payload); + + iterableContainsTheExpectedItems(iterable, expected); + } + + @ParameterizedTest + @CsvSource({ + // True examples + "HP:1, HP:010, true", + "HP:1, HP:011, true", + "HP:1, HP:0110, true", + "HP:01, HP:0110, true", + + "HP:1, HP:020, true", + "HP:1, HP:021, true", + "HP:1, HP:022, true", + + // False examples + "HP:1, HP:1, false", + "HP:01, HP:1, false", + }) + public void isAncestorOf(TermId subject, TermId object, boolean expected) { + assertThat(graph.isAncestorOf(subject, object), equalTo(expected)); + } + + @Test + public void isAncestorOfUnknownSource() { + TermId ok = TermId.of("HP:01"); + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.isAncestorOf(ok, UNKNOWN)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + + assertThat(graph.isAncestorOf(UNKNOWN, ok), equalTo(false)); + } + + @Test + public void getAncestorsUnknownSource() { + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.getAncestors(UNKNOWN, false)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + } - assertThat(GRAPH.isParentOf(UNKNOWN, ok), equalTo(false)); } - @ParameterizedTest - @CsvSource({ - // True examples - "HP:1, HP:010, true", - "HP:1, HP:011, true", - "HP:1, HP:0110, true", - "HP:01, HP:0110, true", - - "HP:1, HP:020, true", - "HP:1, HP:021, true", - "HP:1, HP:022, true", - - // False examples - "HP:1, HP:1, false", - "HP:01, HP:1, false", - }) - public void isAncestorOf(TermId subject, TermId object, boolean expected) { - assertThat(GRAPH.isAncestorOf(subject, object), equalTo(expected)); + @Nested + public class TraversalTests { + @ParameterizedTest + @CsvSource({ + // True examples + "HP:010, HP:1, true", + "HP:011, HP:1, true", + "HP:0110, HP:1, true", + "HP:0110, HP:01, true", + + "HP:020, HP:1, true", + "HP:021, HP:1, true", + "HP:022, HP:1, true", + + // False examples + "HP:1, HP:1, false", + "HP:1, HP:01, false", + }) + public void existsPath(TermId subject, TermId object, boolean expected) { + assertThat(graph.existsPath(subject, object), equalTo(expected)); + } + + @ParameterizedTest + @CsvSource({ + "HP:1, false", + + "HP:01, false", + "HP:010, false", + "HP:011, false", + "HP:0110, true", + + "HP:02, false", + "HP:020, true", + "HP:021, true", + "HP:022, true", + + "HP:03, true", + }) + public void isLeaf(TermId source, boolean expected) { + assertThat(graph.isLeaf(source), equalTo(expected)); + } + + @Test + public void isLeaf_unknownSource() { + NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> graph.isLeaf(UNKNOWN)); + assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + } + } + @Test + public void iterator() { + List expected = Stream.of( + "HP:01", "HP:010", "HP:011", "HP:0110", + "HP:02", "HP:020", "HP:021", "HP:022", + "HP:03", "HP:1") + .map(TermId::of) + .collect(Collectors.toList()); + + iterableContainsTheExpectedItems(graph, expected); + } @Test - public void isAncestorOfUnknownSource() { - TermId ok = TermId.of("HP:01"); - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.isAncestorOf(ok, UNKNOWN)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); + public void size() { + assertThat(graph.size(), is(10)); + } - assertThat(GRAPH.isAncestorOf(UNKNOWN, ok), equalTo(false)); + private Set parsePayload(String payload) { + return payload.isBlank() + ? new HashSet<>() + : Arrays.stream(payload.split(";")) + .map(String::trim) + .map(TermId::of) + .collect(Collectors.toCollection(HashSet::new)); } - @ParameterizedTest - @CsvSource({ - // True examples - "HP:010, HP:1, true", - "HP:011, HP:1, true", - "HP:0110, HP:1, true", - "HP:0110, HP:01, true", - - "HP:020, HP:1, true", - "HP:021, HP:1, true", - "HP:022, HP:1, true", - - // False examples - "HP:1, HP:1, false", - "HP:1, HP:01, false", - }) - public void existsPath(TermId subject, TermId object, boolean expected) { - assertThat(GRAPH.existsPath(subject, object), equalTo(expected)); + /** + * Test that the {@code iterator} yields a sequence of unique elements from the {@code expected} collection. + */ + private static void iterableContainsTheExpectedItems(Iterable iterable, Collection expected) { + List list = new ArrayList<>(); + iterable.forEach(list::add); + HashSet set = new HashSet<>(list); + assertThat("The iterator must yield no duplicates", set.size(), equalTo(list.size())); + assertTrue(set.containsAll(expected)); + assertThat(set, hasSize(expected.size())); + + // Now let's check one more time to ensure the iterable can be actually iterated over >1 times! + list.clear(); + iterable.forEach(list::add); + assertThat("The iterator must yield no duplicates", set.size(), equalTo(list.size())); + assertTrue(set.containsAll(expected)); + assertThat(set, hasSize(expected.size())); } } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java new file mode 100644 index 000000000..2de077c0d --- /dev/null +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java @@ -0,0 +1,91 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import org.junit.jupiter.api.Test; +import org.monarchinitiative.phenol.graph.OntologyGraphEdges; +import org.monarchinitiative.phenol.ontology.data.TermId; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +/** + * Check that {@link CsrMonoOntologyGraphBuilder} builds the expected {@link CsrMonoOntologyGraph} from known input. + */ +public class CsrMonoOntologyGraphBuilderTest { + + private static final TermId ROOT = OntologyGraphEdges.HP1; + + @Test + public void build() { + CsrMonoOntologyGraphBuilder builder = CsrMonoOntologyGraphBuilder.builder(); + CsrMonoOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.hierarchyEdges()); + + assertThat(graph, is(notNullValue(CsrMonoOntologyGraph.class))); + assertThat(graph.root(), equalTo(ROOT)); + } + + @Test + public void checkIndptrs() { + CsrMonoOntologyGraphBuilder builder = CsrMonoOntologyGraphBuilder.builder(); + CsrMonoOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.hierarchyEdges()); + + assertThat(graph.getParentArray().getIndptr(), equalTo(new int[]{0, 1, 2, 3, 5, 6, 7, 8, 9, 10, 10})); + assertThat(graph.getChildArray().getIndptr(), equalTo(new int[]{0, 2, 3, 4, 4, 7, 7, 7, 7, 7, 10})); + } + + @Test + public void checkData() { + CsrMonoOntologyGraphBuilder builder = CsrMonoOntologyGraphBuilder.builder(); + CsrMonoOntologyGraph graph = builder.build(ROOT, OntologyGraphEdges.hierarchyEdges()); + + /* + Checking data is more tricky, since we must check that we have groups of term IDs in particular column. + This is enough because The OntologyGraph API does not require us to provide child/parent termIDs in + any particular order. So, we must not test the `StaticCsrArray.getData()` directly. + Instead, we test if we find expected parents/children in the column. + */ + + String[][] expectedParents = { + /* HP:01 */ {"HP:1"}, + /* HP:010 */ {"HP:01"}, + /* HP:011 */ {"HP:01"}, + /* HP:0110 */ {"HP:010", "HP:011"}, + /* HP:02 */ {"HP:1"}, + /* HP:020 */ {"HP:02"}, + /* HP:021 */ {"HP:02"}, + /* HP:022 */ {"HP:02"}, + /* HP:03 */ {"HP:1"}, + /* HP:1 */ {}, + }; + assertThatGroupsAreConsistent(graph.getNodes(), graph.getParentArray(), expectedParents); + + String[][] expectedChildren = { + /* HP:01 */ {"HP:010", "HP:011"}, + /* HP:010 */ {"HP:0110"}, + /* HP:011 */ {"HP:0110"}, + /* HP:0110 */ {}, + /* HP:02 */ {"HP:020", "HP:021", "HP:022"}, + /* HP:020 */ {}, + /* HP:021 */ {}, + /* HP:022 */ {}, + /* HP:03 */ {}, + /* HP:1 */ {"HP:01", "HP:02", "HP:03"}, + }; + assertThatGroupsAreConsistent(graph.getNodes(), graph.getChildArray(), expectedChildren); + } + + private void assertThatGroupsAreConsistent(TermId[] nodes, + StaticCsrArray array, + String[][] expectedCuries) { + int[] indptr = array.getIndptr(); + TermId[] data = array.getData(); + for (int i = 0; i < nodes.length; i++) { + int start = indptr[i]; + int end = indptr[i + 1]; + String[] curies = expectedCuries[i]; + for (int j = start; j < end; j++) { + String query = data[j].getValue(); + assertThat(curies, hasItemInArray(query)); + } + } + } +} diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphTest.java new file mode 100644 index 000000000..b938d6b6f --- /dev/null +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphTest.java @@ -0,0 +1,19 @@ +package org.monarchinitiative.phenol.graph.csr.mono; + +import org.monarchinitiative.phenol.graph.OntologyGraph; +import org.monarchinitiative.phenol.graph.OntologyGraphEdges; +import org.monarchinitiative.phenol.graph.OntologyGraphTest; +import org.monarchinitiative.phenol.ontology.data.TermId; + +/** + * Check that we can use {@link CsrMonoOntologyGraph} as an {@link OntologyGraph}. + */ +public class CsrMonoOntologyGraphTest extends OntologyGraphTest { + + @Override + protected CsrMonoOntologyGraph getGraph() { + CsrMonoOntologyGraphBuilder builder = CsrMonoOntologyGraphBuilder.builder(); + return builder.build(OntologyGraphEdges.HP1, OntologyGraphEdges.hierarchyEdges()); + } + +} diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrDatasets.java similarity index 81% rename from phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java rename to phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrDatasets.java index 9ef29a409..75b88f08e 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/CsrDatasets.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrDatasets.java @@ -1,9 +1,9 @@ -package org.monarchinitiative.phenol.graph.csr; +package org.monarchinitiative.phenol.graph.csr.poly; /** * Some test data for testing the CSR matrix logic. */ -public class CsrDatasets { +class CsrDatasets { /** * 

@@ -13,7 +13,7 @@ public class CsrDatasets {
    *  [124 126 128 130]]
    *
*/ - public static CsrData full() { + static CsrData full() { return new CsrData<>( new int[]{0, 4, 8, 12, 16}, new int[]{0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3}, @@ -29,7 +29,7 @@ public static CsrData full() { * [5 0 0 6]] * */ - public static CsrData allEdges() { + static CsrData allEdges() { return new CsrData<>( new int[]{0, 2, 3, 4, 6}, new int[]{0, 3, 1, 2, 0, 3}, @@ -44,7 +44,7 @@ public static CsrData allEdges() { * [0 0 0]] * */ - public static CsrData zeroes() { + static CsrData zeroes() { return new CsrData<>( new int[]{0, 0, 0, 0}, new int[]{}, @@ -58,7 +58,7 @@ public static CsrData zeroes() { * [3 0 4 5]] * */ - public static CsrData rect() { + static CsrData rect() { return new CsrData<>( new int[]{0, 2, 5}, new int[]{1, 3, 0, 2, 3}, diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java index fa246333a..857041df9 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/CsrPolyOntologyGraphTest.java @@ -1,240 +1,41 @@ package org.monarchinitiative.phenol.graph.csr.poly; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; -import org.monarchinitiative.phenol.graph.BaseOntologyGraphTest; -import org.monarchinitiative.phenol.graph.NodeNotPresentInGraphException; +import org.monarchinitiative.phenol.graph.OntologyGraph; +import org.monarchinitiative.phenol.graph.OntologyGraphEdges; +import org.monarchinitiative.phenol.graph.OntologyGraphTest; import org.monarchinitiative.phenol.ontology.data.TermId; -import java.util.*; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import static org.junit.jupiter.api.Assertions.*; -import static org.hamcrest.MatcherAssert.*; -import static org.hamcrest.Matchers.*; - -public class CsrPolyOntologyGraphTest extends BaseOntologyGraphTest { - - @Test - public void root() { - TermId root = GRAPH.root(); - assertThat(root.getValue(), equalTo("HP:1")); +/** + * Check that we can use {@link CsrPolyOntologyGraph} as an {@link OntologyGraph}. + */ +public class CsrPolyOntologyGraphTest extends OntologyGraphTest { + +// private static final byte C = 0b01; // child +// private static final byte P = 0b10; // parent +// +// TermId root = TermId.of("HP:1"); +// +// TermId[] nodes = Stream.of( +// "HP:01", "HP:010", "HP:011", "HP:0110", +// "HP:02", "HP:020", "HP:021", "HP:022", +// "HP:03", "HP:1") +// .map(TermId::of) +// .toArray(TermId[]::new); +// +// int[] indptr = new int[] {0, 3, 5, 7, 9, 13, 14, 15, 16, 17, 20}; +// int[] indices = new int[] {1, 2, 9, 0, 3, 0, 3, 1, 2, 5, 6, 7, 9, 4, 4, 4, 9, 0, 4, 8}; +// Byte[] data = new Byte[] {C, C, P, P, C, P, C, P, P, C, C, C, P, P, P, P, P, C, C, C}; +// StaticCsrArray am = new StaticCsrArray<>(indptr, indices, data); +// Predicate hierarchy = b -> (b & P) > 0; +// Predicate hierarchyInverted = b -> (b & C) > 0; +// +// return new CsrPolyOntologyGraph<>(root, nodes, am, TermId::compareTo, hierarchy, hierarchyInverted); + + @Override + protected OntologyGraph getGraph() { + return CsrPolyOntologyGraphBuilder.builder(Byte.class) + .build(OntologyGraphEdges.HP1, OntologyGraphEdges.hierarchyEdges()); } - @Nested - public class TraversalTests { - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:01;HP:02;HP:03", - "HP:02, HP:020;HP:021;HP:022", - "HP:03, ''", - }) - public void getChildren(TermId source, String payload) { - Iterable iterable = GRAPH.getChildren(source, false); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:1;HP:01;HP:02;HP:03", - "HP:02, HP:02;HP:020;HP:021;HP:022", - "HP:03, HP:03", - }) - public void getChildrenIncludingTheSource(TermId source, String payload) { - Iterable iterable = GRAPH.getChildren(source, true); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @Test - public void getChildrenUnknownSource() { - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.getChildren(UNKNOWN, false)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:01;HP:010;HP:011;HP:0110; HP:02;HP:020;HP:021;HP:022; HP:03", - "HP:01, HP:010;HP:011;HP:0110", - "HP:010, HP:0110", - "HP:011, HP:0110", - "HP:0110, ''", - - "HP:02, HP:020;HP:021;HP:022", - "HP:020, ''", - "HP:021, ''", - "HP:022, ''", - "HP:03, ''", - }) - public void getDescendants(TermId source, String payload) { - Iterable iterable = GRAPH.getDescendants(source, false); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:1;HP:01;HP:010;HP:011;HP:0110; HP:02;HP:020;HP:021;HP:022; HP:03", - "HP:01, HP:01;HP:010;HP:011;HP:0110", - "HP:010, HP:010;HP:0110", - "HP:011, HP:011;HP:0110", - "HP:0110, HP:0110", - - "HP:02, HP:02;HP:020;HP:021;HP:022", - "HP:020, HP:020", - "HP:021, HP:021", - "HP:022, HP:022", - "HP:03, HP:03", - }) - public void getDescendantsIncludingTheSource(TermId source, String payload) { - Iterable iterable = GRAPH.getDescendants(source, true); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @Test - public void getDescendantsUnknownSource() { - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.getDescendants(UNKNOWN, false)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, ''", - "HP:01, HP:1", - "HP:03, HP:1", - "HP:0110, HP:010;HP:011", - }) - public void getParents(TermId source, String payload) { - Iterable iterable = GRAPH.getParents(source, false); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:1", - "HP:01, HP:01;HP:1", - "HP:03, HP:03;HP:1", - "HP:0110, HP:0110;HP:010;HP:011", - }) - public void getParentsIncludingTheSource(TermId source, String payload) { - Iterable iterable = GRAPH.getParents(source, true); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @Test - public void getParentsUnknownSource() { - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.getParents(UNKNOWN, false)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, ''", - "HP:01, HP:1", - "HP:0110, HP:010;HP:011;HP:01;HP:1", - "HP:022, HP:02;HP:1", - "HP:03, HP:1", - }) - public void getAncestors(TermId source, String payload) { - Iterable iterable = GRAPH.getAncestors(source, false); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @ParameterizedTest - @CsvSource({ - "HP:1, HP:1", - "HP:01, HP:01;HP:1", - "HP:0110, HP:0110;HP:010;HP:011;HP:01;HP:1", - "HP:022, HP:022;HP:02;HP:1", - "HP:03, HP:03;HP:1", - }) - public void getAncestorsIncludingTheSource(TermId source, String payload) { - Iterable iterable = GRAPH.getAncestors(source, true); - Set expected = parsePayload(payload); - - iterableContainsTheExpectedItems(iterable, expected); - } - - @Test - public void getAncestorsUnknownSource() { - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.getAncestors(UNKNOWN, false)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - } - - private Set parsePayload(String payload) { - return payload.isBlank() - ? new HashSet<>() - : Arrays.stream(payload.split(";")) - .map(String::trim) - .map(TermId::of) - .collect(Collectors.toCollection(HashSet::new)); - } - - } - - @ParameterizedTest - @CsvSource({ - "HP:1, false", - - "HP:01, false", - "HP:010, false", - "HP:011, false", - "HP:0110, true", - - "HP:02, false", - "HP:020, true", - "HP:021, true", - "HP:022, true", - - "HP:03, true", - }) - public void isLeaf(TermId source, boolean expected) { - assertThat(GRAPH.isLeaf(source), equalTo(expected)); - } - - @Test - public void isLeaf_unknownSource() { - NodeNotPresentInGraphException e = assertThrows(NodeNotPresentInGraphException.class, () -> GRAPH.isLeaf(UNKNOWN)); - assertThat(e.getMessage(), equalTo("Item not found in the graph: HP:999")); - } - - @Test - public void iterator() { - List expected = Stream.of( - "HP:01", "HP:010", "HP:011", "HP:0110", - "HP:02", "HP:020", "HP:021", "HP:022", - "HP:03", "HP:1") - .map(TermId::of) - .collect(Collectors.toList()); - - iterableContainsTheExpectedItems(GRAPH, expected); - } - - /** - * Test that the {@code iterator} yields a sequence of unique elements from the {@code expected} collection. - */ - private static void iterableContainsTheExpectedItems(Iterable iterable, Collection expected) { - List list = new ArrayList<>(); - iterable.forEach(list::add); - HashSet set = new HashSet<>(list); - assertThat("The iterator must yield no duplicates", set.size(), equalTo(list.size())); - assertTrue(set.containsAll(expected)); - assertThat(set, hasSize(expected.size())); - } } diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java index a79be2a0c..786697d00 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/poly/StaticCsrArrayTest.java @@ -3,8 +3,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -import org.monarchinitiative.phenol.graph.csr.CsrData; -import org.monarchinitiative.phenol.graph.csr.CsrDatasets; import java.util.*; import java.util.stream.Collectors; From 094eaff21fc2fed75d6cca489d74385c154b187b Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 20:34:45 -0400 Subject: [PATCH 4/9] Expose mono CSR builder through `OntologyGraphBuilders`. --- .../phenol/graph/OntologyGraphBuilders.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java index 0802de62b..69527402e 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/OntologyGraphBuilders.java @@ -1,6 +1,7 @@ package org.monarchinitiative.phenol.graph; +import org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraphBuilder; import org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraphBuilder; import org.monarchinitiative.phenol.ontology.data.TermId; @@ -34,4 +35,12 @@ public static OntologyGraphBuilder csrBuilder(Class clz) { return CsrPolyOntologyGraphBuilder.builder(clz); } + /** + * Get an {@link OntologyGraphBuilder} for building a simple graph with one edge type backed by a CSR-like + * data structure. + */ + public static OntologyGraphBuilder monoCsrBuilder() { + return CsrMonoOntologyGraphBuilder.builder(); + } + } From 6a470a0f692a26bc8c761078965b72f3b1b284e9 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 21:14:26 -0400 Subject: [PATCH 5/9] Setup benchmark for poly vs. mono. Use mono by default in `MinimalOntologyLoader` and `OntologyLoader`. --- .../ontology/data/ImmutableOntology.java | 2 +- .../data/impl/SimpleMinimalOntology.java | 43 ++++++++++++++++++- .../phenol/io/MinimalOntologyLoader.java | 2 + .../phenol/io/OntologyLoader.java | 1 + .../{ => utils}/OntologyLoadingRoutines.java | 10 +++-- ...up.java => CsrMonoOntologyGraphSetup.java} | 8 ++-- .../io/benches/CsrPolyOntologyGraphSetup.java | 41 ++++++++++++++++++ .../phenol/io/benches/OntologyGraphBench.java | 18 +++++++- .../phenol/io/benches/OntologyGraphSetup.java | 34 +++++++++++++++ 9 files changed, 147 insertions(+), 12 deletions(-) rename phenol-io/src/main/java/org/monarchinitiative/phenol/io/{ => utils}/OntologyLoadingRoutines.java (87%) rename phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/{PhenolOntologySetup.java => CsrMonoOntologyGraphSetup.java} (81%) create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrPolyOntologyGraphSetup.java create mode 100644 phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphSetup.java diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/ImmutableOntology.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/ImmutableOntology.java index 4cfe8403f..21e0cfd5c 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/ImmutableOntology.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/ImmutableOntology.java @@ -303,7 +303,7 @@ public ImmutableOntology build() { RelationshipType isA = RelationshipType.IS_A; TermId rootId = OntologyUtils.findRootTermId(terms, relationships, () -> isA); - OntologyGraph ontologyGraph = OntologyGraphBuilders.csrBuilder(Long.class) + OntologyGraph ontologyGraph = OntologyGraphBuilders.monoCsrBuilder() .hierarchyRelation(isA) .build(rootId, relationships); diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java index f30f27c61..b68f15c20 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java @@ -3,6 +3,7 @@ import org.jgrapht.graph.DefaultDirectedGraph; import org.monarchinitiative.phenol.graph.IdLabeledEdge; import org.monarchinitiative.phenol.graph.OntologyGraph; +import org.monarchinitiative.phenol.graph.OntologyGraphBuilder; import org.monarchinitiative.phenol.graph.OntologyGraphBuilders; import org.monarchinitiative.phenol.graph.util.CompatibilityChecker; import org.monarchinitiative.phenol.ontology.data.*; @@ -149,6 +150,23 @@ public static class Builder { private final List terms = new ArrayList<>(); private final List relationships = new ArrayList<>(); private boolean forceBuild = false; + private GraphImplementation graphImplementation = GraphImplementation.MONO; + + /** + * Enum to choose the available {@link OntologyGraph} implementation. + */ + public enum GraphImplementation { + + /** + * Uses {@link OntologyGraphBuilders#csrBuilder(Class)} to build the graph. + */ + POLY, + + /** + * Uses {@link OntologyGraphBuilders#monoCsrBuilder()} to build the graph. + */ + MONO + } private Builder() {} @@ -209,6 +227,17 @@ public Builder forceBuild(boolean value) { return this; } + /** + * Set the graph implementation to be used. + * + * @param graphImplementation the graph implementation to use. + * @return the builder. + */ + public Builder graphImplementation(GraphImplementation graphImplementation) { + this.graphImplementation = Objects.requireNonNull(graphImplementation); + return this; + } + /** * Build the ontology from the provided {@code metaInfo}, {@code terms}, and {@code relationships}. * @return the built {@link SimpleMinimalOntology}. @@ -254,8 +283,18 @@ public SimpleMinimalOntology build() { } // Build the graph. - OntologyGraph ontologyGraph = OntologyGraphBuilders.csrBuilder(Long.class) - .hierarchyRelation(hierarchyRelationshipType) + OntologyGraphBuilder graphBuilder; + switch (graphImplementation) { + case MONO: + graphBuilder = OntologyGraphBuilders.monoCsrBuilder(); + break; + case POLY: + graphBuilder = OntologyGraphBuilders.csrBuilder(Long.class); + break; + default: + throw new IllegalArgumentException(); + } + OntologyGraph ontologyGraph = graphBuilder.hierarchyRelation(hierarchyRelationshipType) .build(rootId, relationships); // Finally, wrap everything into the ontology! diff --git a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/MinimalOntologyLoader.java b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/MinimalOntologyLoader.java index 1bff7511a..5989c27d4 100644 --- a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/MinimalOntologyLoader.java +++ b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/MinimalOntologyLoader.java @@ -4,6 +4,7 @@ import org.monarchinitiative.phenol.io.obographs.OboGraphDocumentAdaptor; import org.monarchinitiative.phenol.io.utils.CurieUtil; import org.monarchinitiative.phenol.io.utils.CurieUtilBuilder; +import org.monarchinitiative.phenol.io.utils.OntologyLoadingRoutines; import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.RelationshipType; import org.monarchinitiative.phenol.ontology.data.impl.SimpleMinimalOntology; @@ -94,6 +95,7 @@ public static MinimalOntology loadOntology(GraphDocument graphDocument, SimpleMinimalOntology ontology = SimpleMinimalOntology.builder() .forceBuild(options.forceBuild()) .hierarchyRelationshipType(RelationshipType.IS_A) + .graphImplementation(SimpleMinimalOntology.Builder.GraphImplementation.MONO) .metaInfo(graphDocumentAdaptor.getMetaInfo()) .terms(graphDocumentAdaptor.getTerms()) .relationships(graphDocumentAdaptor.getRelationships()) diff --git a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoader.java b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoader.java index 26e6b3f05..859919a9f 100644 --- a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoader.java +++ b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoader.java @@ -4,6 +4,7 @@ import org.monarchinitiative.phenol.io.obographs.OboGraphDocumentAdaptor; import org.monarchinitiative.phenol.io.utils.CurieUtil; import org.monarchinitiative.phenol.io.utils.CurieUtilBuilder; +import org.monarchinitiative.phenol.io.utils.OntologyLoadingRoutines; import org.monarchinitiative.phenol.ontology.data.ImmutableOntology; import org.monarchinitiative.phenol.ontology.data.Ontology; import org.slf4j.Logger; diff --git a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoadingRoutines.java b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/utils/OntologyLoadingRoutines.java similarity index 87% rename from phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoadingRoutines.java rename to phenol-io/src/main/java/org/monarchinitiative/phenol/io/utils/OntologyLoadingRoutines.java index b94caaa98..860113090 100644 --- a/phenol-io/src/main/java/org/monarchinitiative/phenol/io/OntologyLoadingRoutines.java +++ b/phenol-io/src/main/java/org/monarchinitiative/phenol/io/utils/OntologyLoadingRoutines.java @@ -1,7 +1,9 @@ -package org.monarchinitiative.phenol.io; +package org.monarchinitiative.phenol.io.utils; import org.geneontology.obographs.core.model.GraphDocument; import org.monarchinitiative.phenol.base.PhenolRuntimeException; +import org.monarchinitiative.phenol.io.MinimalOntologyLoader; +import org.monarchinitiative.phenol.io.OntologyLoader; import org.monarchinitiative.phenol.io.obographs.OboGraphDocumentLoader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -14,13 +16,13 @@ * @see OntologyLoader * @see MinimalOntologyLoader */ -class OntologyLoadingRoutines { +public class OntologyLoadingRoutines { private static final Logger logger = LoggerFactory.getLogger(OntologyLoadingRoutines.class); private OntologyLoadingRoutines(){} - static GraphDocument loadGraphDocument(File file) { + public static GraphDocument loadGraphDocument(File file) { try (InputStream is = new BufferedInputStream(new FileInputStream(file))){ return loadGraphDocument(is); } catch (IOException e) { @@ -28,7 +30,7 @@ static GraphDocument loadGraphDocument(File file) { } } - static GraphDocument loadGraphDocument(InputStream inputStream) { + public static GraphDocument loadGraphDocument(InputStream inputStream) { // The input file might be json or obo/owl. Try to make an educated guess. try (InputStream bufferedStream = new BufferedInputStream(inputStream)) { int readlimit = 16; diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrMonoOntologyGraphSetup.java similarity index 81% rename from phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java rename to phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrMonoOntologyGraphSetup.java index b87c710e0..ea94d48c5 100644 --- a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/PhenolOntologySetup.java +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrMonoOntologyGraphSetup.java @@ -1,9 +1,9 @@ package org.monarchinitiative.phenol.io.benches; -import org.monarchinitiative.phenol.io.MinimalOntologyLoader; import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.Term; import org.monarchinitiative.phenol.ontology.data.TermId; +import org.monarchinitiative.phenol.ontology.data.impl.SimpleMinimalOntology; import org.openjdk.jmh.annotations.Level; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; @@ -17,7 +17,7 @@ @State(Scope.Thread) -public class PhenolOntologySetup { +public class CsrMonoOntologyGraphSetup extends OntologyGraphSetup { private static final Random RANDOM = new Random(500); @@ -27,7 +27,9 @@ public class PhenolOntologySetup { @Setup(Level.Trial) public void prepare() { Path hpoPath = SetupHpo.hpoJsonPath(Constants.HPO_VERSION); - ontology = MinimalOntologyLoader.loadOntology(hpoPath.toFile()); + + ontology = loadOntology(hpoPath, SimpleMinimalOntology.Builder.GraphImplementation.MONO); + primaryTermIds = ontology.getTerms().stream() .map(Term::id) .collect(Collectors.toList()); diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrPolyOntologyGraphSetup.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrPolyOntologyGraphSetup.java new file mode 100644 index 000000000..608279d0a --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/CsrPolyOntologyGraphSetup.java @@ -0,0 +1,41 @@ +package org.monarchinitiative.phenol.io.benches; + +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.monarchinitiative.phenol.ontology.data.Term; +import org.monarchinitiative.phenol.ontology.data.TermId; +import org.monarchinitiative.phenol.ontology.data.impl.SimpleMinimalOntology; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; +import java.util.Random; +import java.util.stream.Collectors; + + +@State(Scope.Thread) +public class CsrPolyOntologyGraphSetup extends OntologyGraphSetup { + + private static final Random RANDOM = new Random(500); + + MinimalOntology ontology; + List primaryTermIds; + + @Setup(Level.Trial) + public void prepare() { + Path hpoPath = SetupHpo.hpoJsonPath(Constants.HPO_VERSION); + + ontology = loadOntology(hpoPath, SimpleMinimalOntology.Builder.GraphImplementation.POLY); + + primaryTermIds = ontology.getTerms().stream() + .map(Term::id) + .collect(Collectors.toList()); + + // ensure the primary term IDs are not sorted if that's what an ontology does. + Collections.shuffle(primaryTermIds, RANDOM); + } + +} diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java index 762b5f470..bc3bcc130 100644 --- a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java @@ -11,14 +11,28 @@ public class OntologyGraphBench { @Benchmark - public void phenol_getParents(PhenolOntologySetup ontology, Blackhole blackhole) { + public void poly_getParents(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { blackhole.consume(ontology.ontology.graph().getParents(termId, false)); } } @Benchmark - public void phenol_getChildren(PhenolOntologySetup ontology, Blackhole blackhole) { + public void poly_getChildren(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + blackhole.consume(ontology.ontology.graph().getChildren(termId, false)); + } + } + + @Benchmark + public void mono_getParents(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + blackhole.consume(ontology.ontology.graph().getParents(termId, false)); + } + } + + @Benchmark + public void mono_getChildren(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { blackhole.consume(ontology.ontology.graph().getChildren(termId, false)); } diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphSetup.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphSetup.java new file mode 100644 index 000000000..3d9ce91bc --- /dev/null +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphSetup.java @@ -0,0 +1,34 @@ +package org.monarchinitiative.phenol.io.benches; + +import org.geneontology.obographs.core.model.GraphDocument; +import org.monarchinitiative.phenol.io.obographs.OboGraphDocumentAdaptor; +import org.monarchinitiative.phenol.io.utils.CurieUtilBuilder; +import org.monarchinitiative.phenol.io.utils.OntologyLoadingRoutines; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.monarchinitiative.phenol.ontology.data.RelationshipType; +import org.monarchinitiative.phenol.ontology.data.impl.SimpleMinimalOntology; + +import java.nio.file.Path; +import java.util.Set; + +public abstract class OntologyGraphSetup { + protected static MinimalOntology loadOntology(Path hpoPath, + SimpleMinimalOntology.Builder.GraphImplementation graphImplementation) { + GraphDocument graphDocument = OntologyLoadingRoutines.loadGraphDocument(hpoPath.toFile()); + + OboGraphDocumentAdaptor graphDocumentAdaptor = OboGraphDocumentAdaptor.builder() + .curieUtil(CurieUtilBuilder.defaultCurieUtil()) + .wantedTermIdPrefixes(Set.of("HP")) + .discardNonPropagatingRelationships(true) + .discardDuplicatedRelationships(true) + .build(graphDocument); + + return SimpleMinimalOntology.builder() + .graphImplementation(graphImplementation) + .metaInfo(graphDocumentAdaptor.getMetaInfo()) + .terms(graphDocumentAdaptor.getTerms()) + .relationships(graphDocumentAdaptor.getRelationships()) + .hierarchyRelationshipType(RelationshipType.IS_A) + .build(); + } +} From 474ffa7477764a9cf2473d7de442353d905694d4 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Tue, 31 Oct 2023 23:01:54 -0400 Subject: [PATCH 6/9] Use hash map instead of binary search. Extend benchmarks. --- .../graph/csr/mono/CsrMonoOntologyGraph.java | 30 ++++----- .../csr/mono/CsrMonoOntologyGraphBuilder.java | 12 ++-- .../mono/CsrMonoOntologyGraphBuilderTest.java | 10 +-- .../phenol/io/benches/OntologyGraphBench.java | 66 +++++++++++++++++-- 4 files changed, 90 insertions(+), 28 deletions(-) diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java index e2b06df79..68274be45 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java @@ -1,7 +1,7 @@ package org.monarchinitiative.phenol.graph.csr.mono; +import org.monarchinitiative.phenol.graph.NodeNotPresentInGraphException; import org.monarchinitiative.phenol.graph.OntologyGraph; -import org.monarchinitiative.phenol.graph.csr.util.Util; import org.monarchinitiative.phenol.utils.IterableIteratorWrapper; import java.util.*; @@ -15,19 +15,16 @@ public class CsrMonoOntologyGraph implements OntologyGraph { private final T root; - private final T[] nodes; - private final Comparator comparator; + private final Map nodesToIdx; private final StaticCsrArray parents; private final StaticCsrArray children; CsrMonoOntologyGraph(T root, - T[] nodes, - Comparator comparator, + Map nodesToIdx, StaticCsrArray parents, StaticCsrArray children) { this.root = Objects.requireNonNull(root); - this.nodes = Objects.requireNonNull(nodes); - this.comparator = Objects.requireNonNull(comparator); + this.nodesToIdx = Objects.requireNonNull(nodesToIdx); this.parents = Objects.requireNonNull(parents); this.children = Objects.requireNonNull(children); } @@ -40,8 +37,11 @@ StaticCsrArray getChildArray() { return children; } - T[] getNodes() { - return nodes; + private int getNodeIdx(T node) { + Integer idx = nodesToIdx.get(node); + if (idx == null) + throw new NodeNotPresentInGraphException(String.format("Item not found in the graph: %s", node)); + return idx; } @Override @@ -57,7 +57,7 @@ public Iterable getChildren(T source, boolean includeSource) { @Override public Iterable getDescendants(T source, boolean includeSource) { // Check if `source` is in the graph. - int intentionallyUnused = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + int intentionallyUnused = getNodeIdx(source); return new IterableIteratorWrapper<>(() -> new TraversingIterator<>(source, src -> getChildren(src, includeSource))); } @@ -70,7 +70,7 @@ public Iterable getParents(T source, boolean includeSource) { @Override public Iterable getAncestors(T source, boolean includeSource) { // Check if `source` is in the graph. - int intentionallyUnused = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + int intentionallyUnused = getNodeIdx(source); return new IterableIteratorWrapper<>(() -> new TraversingIterator<>(source, src -> getParents(src, includeSource))); } @@ -78,7 +78,8 @@ public Iterable getAncestors(T source, boolean includeSource) { private Iterable getImmediateNeighbors(StaticCsrArray array, T source, boolean includeSource) { - int index = Util.getIndexOfUsingBinarySearch(source, nodes, comparator); + int index = getNodeIdx(source); + Set nodes = array.getOutgoingNodes(index); return includeSource @@ -88,13 +89,12 @@ private Iterable getImmediateNeighbors(StaticCsrArray array, @Override public int size() { - return nodes.length; + return nodesToIdx.size(); } @Override public Iterator iterator() { - // TODO - can we do better here? - return Arrays.stream(nodes).iterator(); + return nodesToIdx.keySet().iterator(); } } diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java index 0ea550606..7314115f7 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java @@ -6,10 +6,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -55,8 +52,13 @@ public CsrMonoOntologyGraph build(TermId root, Collection csrData = makeCsrData(nodes, hierarchyEdges); + Map nodeToIdx = new HashMap<>(); + for (int i = 0; i < nodes.length; i++) { + TermId node = nodes[i]; + nodeToIdx.put(node, i); + } - return new CsrMonoOntologyGraph<>(root, nodes, TermId::compareTo, csrData.getParents(), csrData.getChildren()); + return new CsrMonoOntologyGraph<>(root, nodeToIdx, csrData.getParents(), csrData.getChildren()); } private CsrData makeCsrData(TermId[] nodes, diff --git a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java index 2de077c0d..fe94610ae 100644 --- a/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java +++ b/phenol-core/src/test/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilderTest.java @@ -56,7 +56,9 @@ public void checkData() { /* HP:03 */ {"HP:1"}, /* HP:1 */ {}, }; - assertThatGroupsAreConsistent(graph.getNodes(), graph.getParentArray(), expectedParents); + + + assertThatGroupsAreConsistent(graph.size(), graph.getParentArray(), expectedParents); String[][] expectedChildren = { /* HP:01 */ {"HP:010", "HP:011"}, @@ -70,15 +72,15 @@ public void checkData() { /* HP:03 */ {}, /* HP:1 */ {"HP:01", "HP:02", "HP:03"}, }; - assertThatGroupsAreConsistent(graph.getNodes(), graph.getChildArray(), expectedChildren); + assertThatGroupsAreConsistent(graph.size(), graph.getChildArray(), expectedChildren); } - private void assertThatGroupsAreConsistent(TermId[] nodes, + private void assertThatGroupsAreConsistent(int size, StaticCsrArray array, String[][] expectedCuries) { int[] indptr = array.getIndptr(); TermId[] data = array.getData(); - for (int i = 0; i < nodes.length; i++) { + for (int i = 0; i < size; i++) { int start = indptr[i]; int end = indptr[i + 1]; String[] curies = expectedCuries[i]; diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java index bc3bcc130..f0bb343cd 100644 --- a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java @@ -13,29 +13,87 @@ public class OntologyGraphBench { @Benchmark public void poly_getParents(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { - blackhole.consume(ontology.ontology.graph().getParents(termId, false)); + ontology.ontology.graph().getParents(termId, false) + .forEach(blackhole::consume); } } @Benchmark public void poly_getChildren(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { - blackhole.consume(ontology.ontology.graph().getChildren(termId, false)); + ontology.ontology.graph().getChildren(termId, false) + .forEach(blackhole::consume); + } + } + + @Benchmark + public void poly_getAncestors(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + ontology.ontology.graph().getAncestors(termId, false) + .forEach(blackhole::consume); + } + } + + @Benchmark + public void poly_getDescendants(CsrPolyOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + ontology.ontology.graph().getDescendants(termId, false) + .forEach(blackhole::consume); } } @Benchmark public void mono_getParents(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { - blackhole.consume(ontology.ontology.graph().getParents(termId, false)); + ontology.ontology.graph().getParents(termId, false) + .forEach(blackhole::consume); } } @Benchmark public void mono_getChildren(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { for (TermId termId : ontology.primaryTermIds) { - blackhole.consume(ontology.ontology.graph().getChildren(termId, false)); + ontology.ontology.graph().getChildren(termId, false) + .forEach(blackhole::consume); } } + @Benchmark + public void mono_getAncestors(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + ontology.ontology.graph().getAncestors(termId, false) + .forEach(blackhole::consume); + } + } + + @Benchmark + public void mono_getDescendants(CsrMonoOntologyGraphSetup ontology, Blackhole blackhole) { + for (TermId termId : ontology.primaryTermIds) { + ontology.ontology.graph().getDescendants(termId, false) + .forEach(blackhole::consume); + } + } + + /* + # Run complete. Total time: 00:33:59 + Commit 6a470a0f + + REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on + why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial + experiments, perform baseline and negative tests that provide experimental control, make sure + the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. + Do not assume the numbers tell you what you want them to tell. + + NOTE: Current JVM experimentally supports Compiler Blackholes, and they are in use. Please exercise + extra caution when trusting the results, look into the generated code to check the benchmark still + works, and factor in a small probability of new VM bugs. Additionally, while comparisons between + different JVMs are already problematic, the performance difference caused by different Blackhole + modes can be very significant. Please make sure you use the consistent Blackhole mode for comparisons. + + Benchmark Mode Cnt Score Error Units + OntologyGraphBench.mono_getChildren thrpt 25 163.773 ± 11.709 ops/s + OntologyGraphBench.mono_getParents thrpt 25 149.663 ± 8.890 ops/s + OntologyGraphBench.poly_getChildren thrpt 25 134.231 ± 7.379 ops/s + OntologyGraphBench.poly_getParents thrpt 25 131.846 ± 5.647 ops/s + */ } From c537d92ae4fb904b8d5ff9f7ee3451a4c3660d50 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Wed, 1 Nov 2023 09:05:52 -0400 Subject: [PATCH 7/9] Add bench results --- .../phenol/io/benches/OntologyGraphBench.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java index f0bb343cd..ef74999e1 100644 --- a/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java +++ b/phenol-io/src/test/java/org/monarchinitiative/phenol/io/benches/OntologyGraphBench.java @@ -8,6 +8,7 @@ * Benchmark different implementations of {@link org.monarchinitiative.phenol.graph.OntologyGraph}. */ @BenchmarkMode(Mode.Throughput) +@Warmup(iterations = 2) public class OntologyGraphBench { @Benchmark @@ -78,22 +79,24 @@ public void mono_getDescendants(CsrMonoOntologyGraphSetup ontology, Blackhole bl # Run complete. Total time: 00:33:59 Commit 6a470a0f - REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on - why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial - experiments, perform baseline and negative tests that provide experimental control, make sure - the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. - Do not assume the numbers tell you what you want them to tell. - - NOTE: Current JVM experimentally supports Compiler Blackholes, and they are in use. Please exercise - extra caution when trusting the results, look into the generated code to check the benchmark still - works, and factor in a small probability of new VM bugs. Additionally, while comparisons between - different JVMs are already problematic, the performance difference caused by different Blackhole - modes can be very significant. Please make sure you use the consistent Blackhole mode for comparisons. - Benchmark Mode Cnt Score Error Units OntologyGraphBench.mono_getChildren thrpt 25 163.773 ± 11.709 ops/s OntologyGraphBench.mono_getParents thrpt 25 149.663 ± 8.890 ops/s OntologyGraphBench.poly_getChildren thrpt 25 134.231 ± 7.379 ops/s OntologyGraphBench.poly_getParents thrpt 25 131.846 ± 5.647 ops/s - */ + + + # Run complete. Total time: 00:48:00 + Commit 474ffa74 + + Benchmark Mode Cnt Score Error Units + OntologyGraphBench.mono_getAncestors thrpt 25 43.154 ± 0.660 ops/s + OntologyGraphBench.mono_getChildren thrpt 25 555.751 ± 129.691 ops/s + OntologyGraphBench.mono_getDescendants thrpt 25 37.129 ± 0.921 ops/s + OntologyGraphBench.mono_getParents thrpt 25 568.041 ± 45.601 ops/s + OntologyGraphBench.poly_getAncestors thrpt 25 18.252 ± 0.686 ops/s + OntologyGraphBench.poly_getChildren thrpt 25 117.166 ± 16.093 ops/s + OntologyGraphBench.poly_getDescendants thrpt 25 31.324 ± 1.547 ops/s + OntologyGraphBench.poly_getParents thrpt 25 124.328 ± 14.620 ops/s + */ } From 4c9538bf02a648da998a30e5583acbd101339712 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Fri, 3 Nov 2023 11:45:59 -0400 Subject: [PATCH 8/9] Set max line length to 120. --- .editorconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/.editorconfig b/.editorconfig index 000988943..883222775 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,5 +1,6 @@ [*] indent_style = space indent_size = 2 +max_line_length = 120 trim_trailing_whitespace = true insert_final_newline = true From 1330cbfe741dcb2be7960cdec83bc4ec865cceb0 Mon Sep 17 00:00:00 2001 From: Daniel Danis Date: Fri, 3 Nov 2023 18:42:10 -0400 Subject: [PATCH 9/9] Improve documentation and logging. --- .../phenol/graph/csr/mono/CsrMonoOntologyGraph.java | 4 ++++ .../phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java | 1 + .../phenol/graph/csr/mono/SetIncludingSource.java | 3 ++- .../phenol/graph/csr/mono/StaticCsrArray.java | 3 ++- .../phenol/graph/csr/mono/package-info.java | 4 ++-- .../org/monarchinitiative/phenol/graph/csr/package-info.java | 5 +++-- .../phenol/ontology/data/impl/SimpleMinimalOntology.java | 4 ++-- 7 files changed, 16 insertions(+), 8 deletions(-) diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java index 68274be45..c621114fd 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraph.java @@ -8,6 +8,10 @@ /** * An {@link OntologyGraph} that only supports one edge type and supports efficient retrieval of parent or child nodes. + *

+ * It maintains a pair of CSR-like structures, {@link StaticCsrArray}, one for getting the parents and the other + * for children of a term. Both arrays are sorted to contain information for a node {@link T} under the same integer + * index. We get an index from a mapping. * * @param type of the term/graph node. * @author Daniel Danis diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java index 7314115f7..2b67d9ea0 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/CsrMonoOntologyGraphBuilder.java @@ -51,6 +51,7 @@ public CsrMonoOntologyGraph build(TermId root, Collection csrData = makeCsrData(nodes, hierarchyEdges); Map nodeToIdx = new HashMap<>(); for (int i = 0; i < nodes.length; i++) { diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java index 66ed3fd12..5c1f9ffc7 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/SetIncludingSource.java @@ -5,7 +5,8 @@ import java.util.Set; /** - * A {@link Set} that contains an item and all elements of the other set. + * A utility implementation of a {@link Set} that contains + * an item, a singular instance of {@link T}, and the {@link T} elements in the other set. * * @param – the type of elements in this set * @author Daniel Danis diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java index 0ee078371..790da972c 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/StaticCsrArray.java @@ -5,7 +5,8 @@ import java.util.*; /** - * A simple array for retrieval of adjacent graph nodes. + * A CSR-like data structure for retrieval of nodes {@link T} that are adjacent to a {@link T} under + * a row index. *

* The array is inspired by CSR format, but it does not need the column indices and stores directly * the {@link T} elements that represent the adjacent nodes. Since we are mostly interested in propagating relationships diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java index b15102061..aed374e3b 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/mono/package-info.java @@ -1,6 +1,6 @@ /** - * {@linkplain org.monarchinitiative.phenol.graph.csr.mono} package provides a CSR implementation - * of {@link org.monarchinitiative.phenol.graph.OntologyGraph} backed by a single edge type. + * The {@linkplain org.monarchinitiative.phenol.graph.csr.mono} package provides a CSR implementation + * of an {@link org.monarchinitiative.phenol.graph.OntologyGraph} backed by a single edge type. * * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraph * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraphBuilder diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java index 8734e1eca..fc8888ee2 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/graph/csr/package-info.java @@ -1,8 +1,9 @@ /** - * A package with CSR implementation of {@link org.monarchinitiative.phenol.graph.OntologyGraph}. + * A package with CSR implementations of {@link org.monarchinitiative.phenol.graph.OntologyGraph}. * + * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraph + * @see org.monarchinitiative.phenol.graph.csr.mono.CsrMonoOntologyGraphBuilder * @see org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraph * @see org.monarchinitiative.phenol.graph.csr.poly.CsrPolyOntologyGraphBuilder - * @see org.monarchinitiative.phenol.graph.csr.poly.StaticCsrArray */ package org.monarchinitiative.phenol.graph.csr; diff --git a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java index b68f15c20..bab04c062 100644 --- a/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java +++ b/phenol-core/src/main/java/org/monarchinitiative/phenol/ontology/data/impl/SimpleMinimalOntology.java @@ -153,7 +153,7 @@ public static class Builder { private GraphImplementation graphImplementation = GraphImplementation.MONO; /** - * Enum to choose the available {@link OntologyGraph} implementation. + * Enum to choose from the {@link OntologyGraph} implementations. */ public enum GraphImplementation { @@ -292,7 +292,7 @@ public SimpleMinimalOntology build() { graphBuilder = OntologyGraphBuilders.csrBuilder(Long.class); break; default: - throw new IllegalArgumentException(); + throw new IllegalArgumentException(String.format("Unsupported graph implementation %s", graphImplementation)); } OntologyGraph ontologyGraph = graphBuilder.hierarchyRelation(hierarchyRelationshipType) .build(rootId, relationships);