Skip to content

fix(vermeer): reject incomplete Louvain output - #371

Merged
imbajin merged 2 commits into
apache:masterfrom
hugegraph:fix/vermeer-louvain-early-output
Nov 3, 2026
Merged

imbajin merged 2 commits into
apache:masterfrom
hugegraph:fix/vermeer-louvain-early-output

Conversation

@imbajin

@imbajin imbajin commented Nov 3, 2026

Copy link
Copy Markdown
Member

What changes

Louvain and weighted Louvain can be stopped before the first iteration completes. Reading output in that state could panic or return intermediate state. Reject output until the first iteration finishes, while preserving cancellation and pause handling.

Validation

  • Targeted Go tests, including race checks, cover early output and cancellation.
  • Real multi-Worker early-stop behavior and deployment remain to be verified.

- reject output before the first pass completes
- preserve cancellation and report actionable errors
- cover output validation and task control

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: yes. Summary: ValidateOutput correctly rejects output while firstStep is true, and the RunOutput reorder keeps cancel and pause handling ahead of validation. vermeer-ci.yml never runs ./algorithms, so the new Louvain test does not run in CI.

- run algorithm tests with the race detector
- include Louvain output regression coverage
- retain existing worker test checks

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: no. Score: 9/10. Summary: ValidateOutput rejects Louvain output while firstStep is true, which only compute.max_step can reach, and RunOutput keeps cancel and pause handling ahead of it; CI now runs ./algorithms and the tests pass at this head.

@imbajin
imbajin merged commit aa2c7ad into apache:master Nov 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants