Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(algorithm): support random walk in computer #274

Merged
merged 12 commits into from
Oct 28, 2023

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 javeme and coderzc 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
7 of 8 checks passed
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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature] Random walk algorithm
3 participants