feat(algorithm): support single source shortest path algorithm #285

diaohancai · 2023-11-21T14:36:21Z

Purpose of the PR

implement feat(algorithm): support single source shortest path algorithm #283

Main Changes

Add single source shortest path algorithm.
There are 3 types of target vertex：

single target:

single_source_shortest_path.source_id="\"A\""
single_source_shortest_path.target_id="\"E\""

multiple target:

single_source_shortest_path.source_id="\"A\""
single_source_shortest_path.target_id="\"E\", \"C\""

all target:

single_source_shortest_path.source_id="\"A\""
single_source_shortest_path.target_id=*

Verifying these changes

Trivial rework / code cleanup without any test coverage. (No Need)
Already covered by existing tests, such as (please modify tests here).
Need tests and can be verified as follows.
Unit test: org.apache.hugegraph.computer.algorithm.path.shortest.SingleSourceShortestPathTest#testRunAlgorithm

Does this PR potentially affect the following parts?

Documentation Status

Doc - TODO
Doc - Done
Doc - No Need

...m/src/main/java/org/apache/hugegraph/computer/algorithm/path/shortest/ShortestPathValue.java

...src/main/java/org/apache/hugegraph/computer/algorithm/path/shortest/ShortestPathMessage.java

...m/src/main/java/org/apache/hugegraph/computer/algorithm/path/shortest/ShortestPathValue.java

computer-api/src/main/java/org/apache/hugegraph/computer/core/graph/value/ListValue.java

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SourceTargetShortestPath.java

...m/src/main/java/org/apache/hugegraph/computer/algorithm/path/shortest/ShortestPathValue.java

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SourceTargetShortestPath.java

codecov · 2023-12-04T12:10:32Z

Codecov Report

Attention: Patch coverage is 68.20276% with 69 lines in your changes are missing coverage. Please review.

Project coverage is 84.97%. Comparing base (ff85e34) to head (d55c7c7).
Report is 13 commits behind head on master.

Files	Patch %	Lines
...orithm/path/shortest/SingleSourceShortestPath.java	62.88%	18 Missing and 18 partials ⚠️
...rg/apache/hugegraph/computer/core/util/IdUtil.java	21.42%	8 Missing and 3 partials ⚠️
...ph/computer/core/combiner/IdListMergeCombiner.java	0.00%	10 Missing ⚠️
...ache/hugegraph/computer/core/util/JsonUtilExt.java	0.00%	6 Missing ⚠️
...he/hugegraph/computer/core/graph/id/IdFactory.java	42.85%	3 Missing and 1 partial ⚠️
...ath/shortest/SingleSourceShortestPathCombiner.java	75.00%	0 Missing and 1 partial ⚠️
...m/path/shortest/SingleSourceShortestPathValue.java	96.87%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #285      +/-   ##
============================================
- Coverage     85.03%   84.97%   -0.07%     
- Complexity     3246     3359     +113     
============================================
  Files           345      360      +15     
  Lines         12298    12678     +380     
  Branches       1102     1149      +47     
============================================
+ Hits          10458    10773     +315     
- Misses         1315     1341      +26     
- Partials        525      564      +39

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

computer-api/src/main/java/org/apache/hugegraph/computer/core/combiner/IdListMergeCombiner.java

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

computer-api/src/main/java/org/apache/hugegraph/computer/core/graph/id/IdFactory.java

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

javeme · 2024-02-03T11:28:10Z

computer-api/src/main/java/org/apache/hugegraph/computer/core/graph/id/IdCategory.java

+
+import org.apache.commons.lang3.StringUtils;
+
+public enum IdCategory {


not sure why not reuse IdType

IdType: LONG, UTF8, UUID.
IdCategory: NUMBER, STRING, UUID.
I suppose that IdType is too technical and IdCategory is more user-friendly.
In fact, they correspond one to one.·

remove IdCategory, reuse IdType if necessary.

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

javeme · 2024-02-03T11:32:10Z

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

+        if (this.targetQuantityType != QuantityType.ALL) {
+            this.targetIdSet = new IdSet();
+            for (String targetIdStr : this.targetIdStr.split(",")) {
+                targetIdSet.add(IdUtil.parseId(this.vertexIdType, targetIdStr));


seems the targets id should be different types? like [111, "abc", "222", 333, U"uuid..."]

javeme · 2024-02-24T05:11:40Z

...java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPathTest.java

@@ -40,8 +40,8 @@ public class SingleSourceShortestPathTest extends AlgorithmTestBase {
    public static final String EL = "road";
    public static final String PROPERTY_KEY = "distance";

-    public static final String SOURCE_ID = "A";
-    public static final String TARGET_ID = "E";
+    public static final String SOURCE_ID = "{\"id\": \"A\", \"idType\": \"string\"}";


we can use id format like this (ref VertexAPI.checkAndParseVertexId()):

String SOURCE_ID = "\"abc\""; String SOURCE_ID = "\"123\""; String SOURCE_ID = "123"; String SOURCE_ID = "U\"uuid-xxxx\"";

javeme · 2024-02-24T05:12:57Z

computer-api/src/main/java/org/apache/hugegraph/computer/core/util/JsonUtilExt.java

+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+// todo move to org.apache.hugegraph.util.JsonUtil later


expect format // TODO: xx

javeme · 2024-02-24T05:18:35Z

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

            vertex.inactivate();
            return;
        }

        if (vertex.numEdges() <= 0) {
            // isolated vertex
-            LOG.debug("source vertex {} can not reach target vertex {}",
-                      this.sourceIdStr, this.targetIdStr);
+            LOG.debug("source vertex {} is isolated", this.sourceId);


this format is more convenient for retrieving logs: The source vertex is isolated: {}

javeme · 2024-02-24T05:20:04Z

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SingleSourceShortestPath.java

@@ -157,17 +158,16 @@ public void compute0(ComputationContext context, Vertex vertex) {

        // single target && source == target
        if (this.targetQuantityType == QuantityType.SINGLE &&
-            this.sourceIdStr.equals(this.targetIdStr)) {
+            this.sourceId.equals(this.targetIdSet.value().iterator().next())) {


add a private method targetIdOne() for this.targetIdSet.value().iterator().next()? we can also check there exist one element in targetIdOne()

javeme · 2024-03-03T04:51:11Z

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

-        Assert.assertEquals(IdType.UUID, IdUtil.parseId(IdCategory.UUID, uuid).idType());
+        String utf81 = "\"abc\"";
+        String utf82 = "\"222\"";
+        String l = "222";


javeme · 2024-03-03T04:51:43Z

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

-        Assert.assertEquals(IdType.LONG, IdUtil.parseId(IdCategory.NUMBER, "222").idType());
-        Assert.assertEquals(IdType.UTF8, IdUtil.parseId(IdCategory.STRING, "aaa222").idType());
-        Assert.assertEquals(IdType.UUID, IdUtil.parseId(IdCategory.UUID, uuid).idType());
+        String utf81 = "\"abc\"";


idUtf8WithString

javeme · 2024-03-03T04:51:59Z

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

-        Assert.assertEquals(IdType.UTF8, IdUtil.parseId(IdCategory.STRING, "aaa222").idType());
-        Assert.assertEquals(IdType.UUID, IdUtil.parseId(IdCategory.UUID, uuid).idType());
+        String utf81 = "\"abc\"";
+        String utf82 = "\"222\"";


idUtf8WithNumber

javeme · 2024-03-03T04:53:34Z

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

+        Assert.assertEquals(IdType.UTF8, IdUtil.parseId(utf81).idType());
+        Assert.assertEquals(IdType.UTF8, IdUtil.parseId(utf82).idType());
+        Assert.assertEquals(IdType.LONG, IdUtil.parseId(l).idType());
+        Assert.assertEquals(IdType.UUID, IdUtil.parseId(uuid).idType());


can we also add some exception cases, like idEmpty/idDouble/uuidInvalid

javeme · 2024-03-03T04:54:56Z

computer-api/src/main/java/org/apache/hugegraph/computer/core/util/IdUtil.java

-        if (idCategory == null) {
-            // automatic inference
-            return parseId(idStr);
+        if (idStr.startsWith("U\"")) {


also move to try-catch?

javeme · 2024-03-03T05:51:51Z

computer-test/src/main/java/org/apache/hugegraph/computer/core/util/IdUtilTest.java

+        Assert.assertThrows(ComputerException.class, () -> {
+            IdUtil.parseId(idNull).idType();
+        });
+        Assert.assertThrows(ComputerException.class, () -> {


also keep IllegalArgumentException?

javeme

LGTM

diaohancai added 6 commits November 21, 2023 15:38

feat: ListValue add clear() method

28691fd

feat: Source Target Shortest Path

d9c5c84

test: test data is complicated

3aff95d

test: ListValue.clear unit test

b6c0f4c

fix: ring loop

f5a5de9

optimize: message combine

4c37b2d

javeme reviewed Nov 22, 2023

View reviewed changes

...m/src/main/java/org/apache/hugegraph/computer/algorithm/path/shortest/ShortestPathValue.java Outdated Show resolved Hide resolved

...ain/java/org/apache/hugegraph/computer/algorithm/path/shortest/SourceTargetShortestPath.java Outdated Show resolved Hide resolved

Merge branch 'master' into pr/285

15f95f6

diaohancai added 4 commits December 26, 2023 17:22

refactor(algorithm): Single Source Shortest Path

6630b82

refactor(algorithm): output json

9524900

feat(algorithm): multiple target optimization

2804bd7

feat(core): IdList Merge Combiner

cdaba23

dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Dec 26, 2023

diaohancai requested a review from javeme December 26, 2023 17:20

diaohancai changed the title ~~feat(algorithm): support source target shortest path algorithm~~ feat(algorithm): support single source shortest path algorithm Dec 26, 2023

javeme reviewed Dec 28, 2023

View reviewed changes

diaohancai added 2 commits December 31, 2023 16:10

chore(algorithm): simple adjustments

125bba6

optimization(algorithm): change reachedTargets from IdList to IdSet

9587802

diaohancai requested a review from javeme January 1, 2024 05:53

javeme reviewed Jan 1, 2024

View reviewed changes

diaohancai added 3 commits January 22, 2024 08:22

chore: json style key

01cde50

improve: convert id from string to ID with type

6b42bf5

chore: add IdUtilTest unit test

55b0ed3

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jan 25, 2024

diaohancai requested a review from javeme January 25, 2024 16:01

javeme reviewed Feb 3, 2024

View reviewed changes

fix: all targets reached

2828bb7

diaohancai added 2 commits February 6, 2024 17:35

improve: remove unnecessary member var

a0a0103

improve: source vertex and target vertex specify idType

c4ca8fe

diaohancai requested a review from javeme February 6, 2024 14:46

chore: get properties

174584e

javeme reviewed Feb 24, 2024

View reviewed changes

diaohancai added 2 commits February 29, 2024 12:59

improve: input vertex id parse

aa8238d

chore: log improvement

0b1c720

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Feb 29, 2024

diaohancai requested a review from javeme February 29, 2024 05:27

javeme reviewed Mar 3, 2024

View reviewed changes

test: apply exception testing

6510424

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Mar 3, 2024

diaohancai requested a review from javeme March 3, 2024 05:43

javeme reviewed Mar 3, 2024

View reviewed changes

test: parse empty id throws IllegalArgumentException

d55c7c7

diaohancai requested a review from javeme March 3, 2024 05:55

javeme approved these changes Mar 3, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 3, 2024

imbajin requested a review from coderzc March 4, 2024 08:17

coderzc approved these changes Mar 12, 2024

View reviewed changes

imbajin merged commit 192ef3b into apache:master Mar 12, 2024
7 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(algorithm): support single source shortest path algorithm #285

feat(algorithm): support single source shortest path algorithm #285

diaohancai commented Nov 21, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

javeme Feb 3, 2024

diaohancai Feb 6, 2024 •

edited

Loading

diaohancai Feb 29, 2024 •

edited

Loading

javeme Feb 3, 2024

javeme Feb 24, 2024

javeme Feb 24, 2024

javeme Feb 24, 2024

javeme Feb 24, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

javeme Mar 3, 2024

javeme left a comment


		import org.apache.commons.lang3.StringUtils;

		public enum IdCategory {

feat(algorithm): support single source shortest path algorithm #285

feat(algorithm): support single source shortest path algorithm #285

Conversation

diaohancai commented Nov 21, 2023 • edited Loading

Purpose of the PR

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

diaohancai Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

diaohancai Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javeme left a comment

Choose a reason for hiding this comment

diaohancai commented Nov 21, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

diaohancai Feb 6, 2024 •

edited

Loading

diaohancai Feb 29, 2024 •

edited

Loading