Merge pull request #6567 from breakanalysis/hashgnn-validation

Add HashGNN validation

Author: breakanalysis

algo/src/main/java/org/neo4j/gds/embeddings/hashgnn/BinarizeFeaturesConfig.java

@@ -22,6 +22,8 @@
 import org.immutables.value.Value;
 import org.neo4j.gds.annotation.Configuration;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
 @Configuration
@@ -39,4 +41,13 @@ default double threshold() {
     default Map<String, Object> toMap() {
         return Map.of(); // Will be overwritten
     }
+
+    @Configuration.CollectKeys
+    @Value.Auxiliary
+    @Value.Default
+    @Value.Parameter(false)
+    default Collection<String> configKeys() {
+        return List.of();
+    }
+
 }

algo/src/main/java/org/neo4j/gds/embeddings/hashgnn/GenerateFeaturesConfig.java

@@ -22,8 +22,12 @@
 import org.immutables.value.Value;
 import org.neo4j.gds.annotation.Configuration;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
+import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
+
 @Configuration
 public interface GenerateFeaturesConfig {
     @Configuration.IntegerRange(min = 1)
@@ -37,4 +41,19 @@ public interface GenerateFeaturesConfig {
     default Map<String, Object> toMap() {
         return Map.of(); // Will be overwritten
     }
+
+    @Configuration.CollectKeys
+    @Value.Auxiliary
+    @Value.Default
+    @Value.Parameter(false)
+    default Collection<String> configKeys() {
+        return List.of();
+    }
+
+    @Value.Check
+    default void validate() {
+        if (densityLevel() > dimension()) {
+            throw new IllegalArgumentException(formatWithLocale("Generate features requires `densityLevel` to be at most `dimension` but was %d > %d.", densityLevel(), dimension()));
+        }
+    }
 }

algo/src/main/java/org/neo4j/gds/embeddings/hashgnn/HashGNNConfig.java

@@ -77,7 +77,10 @@ static Optional<BinarizeFeaturesConfig> parseBinarizationConfig(Object o) {
         if (o instanceof Optional) {
             return (Optional<BinarizeFeaturesConfig>) o;
         }
-        return Optional.of(new BinarizeFeaturesConfigImpl(CypherMapWrapper.create((Map<String, Object>) o)));
+        var cypherMapWrapper = CypherMapWrapper.create((Map<String, Object>) o);
+        var binarizeFeaturesConfig = new BinarizeFeaturesConfigImpl(cypherMapWrapper);
+        cypherMapWrapper.requireOnlyKeysFrom(binarizeFeaturesConfig.configKeys());
+        return Optional.of(binarizeFeaturesConfig);
     }
 
     static Map<String, Object> toMapBinarizationConfig(BinarizeFeaturesConfig config) {
@@ -88,10 +91,14 @@ static Optional<GenerateFeaturesConfig> parseGenerateFeaturesConfig(Object o) {
         if (o instanceof Optional) {
             return (Optional<GenerateFeaturesConfig>) o;
         }
-        return Optional.of(new GenerateFeaturesConfigImpl(CypherMapWrapper.create((Map<String, Object>) o)));
+        var cypherMapWrapper = CypherMapWrapper.create((Map<String, Object>) o);
+        var generateFeaturesConfig = new GenerateFeaturesConfigImpl(cypherMapWrapper);
+        cypherMapWrapper.requireOnlyKeysFrom(generateFeaturesConfig.configKeys());
+        return Optional.of(generateFeaturesConfig);
     }
 
     static Map<String, Object> toMapGenerateFeaturesConfig(GenerateFeaturesConfig config) {
         return config.toMap();
     }
+
 }

algo/src/main/java/org/neo4j/gds/embeddings/hashgnn/HashGNNMutateConfig.java

@@ -29,4 +29,5 @@ public interface HashGNNMutateConfig extends HashGNNConfig, MutatePropertyConfig
     static HashGNNMutateConfig of(CypherMapWrapper config) {
         return new HashGNNMutateConfigImpl(config);
     }
+
 }

algo/src/main/java/org/neo4j/gds/embeddings/hashgnn/RawFeaturesTask.java

@@ -34,8 +34,11 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
+import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
+
 class RawFeaturesTask implements Runnable {
     private final Partition partition;
+    private final Graph graph;
     private final List<FeatureExtractor> featureExtractors;
     private final int inputDimension;
     private final HugeObjectArray<HugeAtomicBitSet> features;
@@ -44,12 +47,14 @@ class RawFeaturesTask implements Runnable {
 
     RawFeaturesTask(
         Partition partition,
+        Graph graph,
         List<FeatureExtractor> featureExtractors,
         int inputDimension,
         HugeObjectArray<HugeAtomicBitSet> features,
         ProgressTracker progressTracker
     ) {
         this.partition = partition;
+        this.graph = graph;
         this.featureExtractors = featureExtractors;
         this.inputDimension = inputDimension;
         this.features = features;
@@ -77,6 +82,7 @@ static HugeObjectArray<HugeAtomicBitSet> compute(
         var tasks = partitions.stream()
             .map(p -> new RawFeaturesTask(
                 p,
+                graph,
                 featureExtractors,
                 inputDimension,
                 features,
@@ -103,16 +109,21 @@ public void run() {
             FeatureExtraction.extract(nodeId, -1, featureExtractors, new FeatureConsumer() {
                 @Override
                 public void acceptScalar(long nodeOffset, int offset, double value) {
-                    if (value != 0.0) {
+                    if (value == 1.0) {
                         nodeFeatures.set(offset);
+                    } else if (value != 0.0) {
+                        throw new IllegalArgumentException(formatWithLocale("Feature properties may only contain values 0 and 1 unless `binarizeFeatures` is used. Node %d and possibly other nodes have a feature property containing value %f", graph.toOriginalNodeId(nodeId), value));
                     }
                 }
 
                 @Override
                 public void acceptArray(long nodeOffset, int offset, double[] values) {
                     for (int inputFeatureOffset = 0; inputFeatureOffset < values.length; inputFeatureOffset++) {
-                        if (values[inputFeatureOffset] != 0.0) {
+                        var value = values[inputFeatureOffset];
+                        if (value == 1.0) {
                             nodeFeatures.set(offset + inputFeatureOffset);
+                        } else if (value != 0.0) {
+                            throw new IllegalArgumentException(formatWithLocale("Feature properties may only contain values 0 and 1 unless `binarizeFeatures` is used. Node %d and possibly other nodes have a feature property containing value %.17f", graph.toOriginalNodeId(nodeId), value));
                         }
                     }
                 }

algo/src/test/java/org/neo4j/gds/embeddings/hashgnn/HashGNNConfigTest.java

@@ -20,6 +20,7 @@
 package org.neo4j.gds.embeddings.hashgnn;
 
 import org.junit.jupiter.api.Test;
+import org.neo4j.gds.core.CypherMapWrapper;
 
 import java.util.List;
 import java.util.Map;
@@ -63,4 +64,48 @@ void requiresFeaturePropertiesIfNoGeneratedFeatures() {
                 .build();
         }).hasMessage("When `generateFeatures` is not given, `featureProperties` must be non-empty.");
     }
+
+    @Test
+    void requiresDensityLevelAtMostDensity() {
+        assertThatThrownBy(() -> {
+            HashGNNConfigImpl
+                .builder()
+                .embeddingDensity(4)
+                .generateFeatures(Map.of("dimension", 4, "densityLevel", 5))
+                .iterations(100)
+                .build();
+        }).hasMessage("Generate features requires `densityLevel` to be at most `dimension` but was 5 > 4.");
+    }
+
+    @Test
+    void failsOnInvalidBinarizationKeys() {
+        assertThatThrownBy(() -> {
+            new HashGNNConfigImpl(CypherMapWrapper.create(
+                Map.of(
+                    "mutateProperty", "foo",
+                    "featureProperties", List.of("x"),
+                    "binarizeFeatures", Map.of("dimension", 100, "treshold", 2.0),
+                    "embeddingDensity", 4,
+                    "iterations", 100
+                )
+            ));
+
+        }).isInstanceOf(IllegalArgumentException.class)
+            .hasMessage("Unexpected configuration key: treshold (Did you mean [threshold]?)");
+    }
+
+    @Test
+    void failsOnInvalidGenerateFeaturesKeys() {
+        assertThatThrownBy(() -> {
+            new HashGNNConfigImpl(CypherMapWrapper.create(
+                Map.of(
+                    "generateFeatures", Map.of("dimension", 100, "densityElfen", 2),
+                    "embeddingDensity", 4,
+                    "iterations", 100
+                )
+            ));
+
+        }).isInstanceOf(IllegalArgumentException.class)
+            .hasMessage("No value specified for the mandatory configuration parameter `densityLevel` (a similar parameter exists: [densityElfen])");
+    }
 }

algo/src/test/java/org/neo4j/gds/embeddings/hashgnn/RawFeaturesTaskTest.java

@@ -34,6 +34,7 @@
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 @GdlExtension
 class RawFeaturesTaskTest {
@@ -45,12 +46,41 @@ class RawFeaturesTaskTest {
         ", (b:N {f1: 1, f2: [1.0, 0.0]})" +
         ", (c:N {f1: 1, f2: [0.0, 1.0]})";
 
+    @GdlGraph(graphNamePrefix = "nonBinary")
+    private static final String NON_BINARY =
+        "CREATE" +
+        "  (a:N {f1: 1, f2: [1.0, 1.0]})" +
+        ", (b:N {f1: 1, f2: [1.0, 0.0000000001]})" +
+        ", (c:N {f1: 1, f2: [0.0, 1.0]})";
+
     @Inject
     private Graph graph;
 
+    @Inject
+    private Graph nonBinaryGraph;
+
     @Inject
     private IdFunction idFunction;
 
+    @Test
+    void shouldFailOnNonBinaryFeatures() {
+        var partition = new Partition(0, nonBinaryGraph.nodeCount());
+        var featureExtractors = FeatureExtraction.propertyExtractors(nonBinaryGraph, List.of("f1", "f2"));
+        var features = HugeObjectArray.newArray(HugeAtomicBitSet.class, nonBinaryGraph.nodeCount());
+        var inputDimension = FeatureExtraction.featureCount(featureExtractors);
+
+        assertThatThrownBy(() -> {
+            new RawFeaturesTask(
+                partition,
+                nonBinaryGraph,
+                featureExtractors,
+                inputDimension,
+                features,
+                ProgressTracker.NULL_TRACKER
+            ).run();
+        }).isInstanceOf(IllegalArgumentException.class)
+            .hasMessage("Feature properties may only contain values 0 and 1 unless `binarizeFeatures` is used. Node 1 and possibly other nodes have a feature property containing value 0.00000000010000000");
+    }
     @Test
     void shouldPickCorrectFeatures() {
         var partition = new Partition(0, graph.nodeCount());
@@ -60,6 +90,7 @@ void shouldPickCorrectFeatures() {
 
         new RawFeaturesTask(
             partition,
+            graph,
             featureExtractors,
             inputDimension,
             features,

doc/modules/ROOT/partials/machine-learning/node-embeddings/hashgnn/specific-configuration.adoc

@@ -4,6 +4,6 @@
 | heterogeneous                                                                    | Boolean         | false           | yes       | Whether different relationship types should be treated differently.
 | neighborInfluence                                                                | Float           | 1.0             | yes       | Controls how often neighbors' features are sampled in each iteration relative to sampling the node's own features. Must be non-negative.
 | binarizeFeatures                                                                 | Map             | n/a             | yes       | A map with keys `dimension` and `threshold`. If given, features are transformed into `dimension` binary features via hyperplane rounding. Increasing `threshold` makes the output more sparse, and it defaults to `0`. The value of `dimension` must be at least 1.
-| generateFeatures                                                                 | Map             | n/a             | yes       | A map with keys `dimension` and `densityLevel`. Should be given if and only if `featureProperties` is empty. If given, `dimension` binary features are generated with approximately `densityLevel` active features per node. Both must be at least 1.
+| generateFeatures                                                                 | Map             | n/a             | yes       | A map with keys `dimension` and `densityLevel`. Should be given if and only if `featureProperties` is empty. If given, `dimension` binary features are generated with approximately `densityLevel` active features per node. Both must be at least 1 and `densityLevel` at most `dimension`.
 | outputDimension                                                                  | Integer         | n/a             | yes       | If given, the embeddings are projected randomly into `outputDimension` dense features. Must be at least 1.
 | randomSeed                                                                       | Integer         | n/a             | yes       | A random seed which is used for all randomness in computing the embeddings.

proc/embeddings/src/test/java/org/neo4j/gds/embeddings/hashgnn/HashGNNStreamProcTest.java

@@ -68,6 +68,5 @@ void shouldComputeNonZeroEmbeddings() {
                 .hasSize(3)
                 .anyMatch(value -> value != 0.0);
         });
-
     }
 }