Skip to content

Conversation

diaohancai
Copy link
Contributor

Purpose of the PR

Main Changes

Add an algorithm: random walk.

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.
    Please run unit test: org.apache.hugegraph.computer.algorithm.sampling.RandomWalkTest#testRunAlgorithm

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need
@imbajin imbajin requested review from coderzc and javeme October 24, 2023 08:00
@imbajin imbajin changed the title Feat algorithm random walk feat(algorithm): support random walk in computer Oct 24, 2023
IdListList curValue = sourceVertex.value();
curValue.add(path.copy());
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line

this.path.add(vertex.id());
}

public Boolean getIsFinish() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer isFinish

return isFinish.value();
}

public void setIsFinish(Boolean isFinish) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer boolean isFinish

public void setIsFinish(Boolean isFinish) {
this.isFinish = new BooleanValue(isFinish);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

}
return propValues;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

this.setIfAbsent(params, RandomWalk.OPTION_WALK_LENGTH,
"3");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

return this.path;
}

public void addPath(Vertex vertex) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer addToPath


@Override
public String name() {
return "randomWalk";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also keep "page_rank" style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok~ Thanks~

@diaohancai diaohancai requested a review from javeme October 25, 2023 01:53
}

public Boolean isFinish() {
return isFinish.value();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to call boolValue() instead of value() here and let isFinish() return boolean type.
and please also keep this.isFinish style.

Comment on lines 39 to 42
this.setIfAbsent(params, RandomWalk.OPTION_WALK_PER_NODE,
"3");
this.setIfAbsent(params, RandomWalk.OPTION_WALK_LENGTH,
"3");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep in one line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok~

@javeme
Copy link
Contributor

javeme commented Oct 25, 2023

note ci error:

computer-algorithm/src/main/java/org/apache/hugegraph/computer/algorithm/sampling/RandomWalk.java:111: Line is longer than 100 characters (found 106). [LineLength]
@diaohancai diaohancai requested a review from javeme October 26, 2023 01:22
@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #274 (565d6e7) into master (a21a191) will decrease coverage by 0.66%.
Report is 2 commits behind head on master.
The diff coverage is 94.28%.

@@ Coverage Diff @@ ## master #274 +/- ## ============================================ - Coverage 85.82% 85.17% -0.66%  - Complexity 3233 3277 +44  ============================================ Files 344 349 +5 Lines 12124 12388 +264 Branches 1092 1112 +20 ============================================ + Hits 10406 10552 +146  - Misses 1193 1305 +112  - Partials 525 531 +6 
Files Coverage Δ
.../computer/algorithm/sampling/RandomWalkOutput.java 100.00% <100.00%> (ø)
.../computer/algorithm/sampling/RandomWalkParams.java 100.00% <100.00%> (ø)
...computer/algorithm/sampling/RandomWalkMessage.java 94.11% <94.11%> (ø)
...egraph/computer/algorithm/sampling/RandomWalk.java 92.06% <92.06%> (ø)

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@coderzc coderzc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@javeme javeme merged commit d55680c into apache:master Oct 28, 2023
imbajin added a commit that referenced this pull request Dec 4, 2023
- implement #279 - follow-up #274 (V1 version) The current random walk algorithm requires 2 additional features. - Biased random walk. - Second order random walk. --------- Co-authored-by: diaohancai <diaohancai@cvte.com> Co-authored-by: imbajin <jin@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants