FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Changes the output label of CC to min of original ID by architch · Pull Request #320 · graphframes/graphframes · GitHub

Changes the output label of CC to min of original ID - #320

Merged
SemyonSinchenko merged 3 commits into
graphframes:masterfrom
architch:CC-original-label
Apr 1, 2025
Merged

Changes the output label of CC to min of original ID#320
SemyonSinchenko merged 3 commits into
graphframes:masterfrom
architch:CC-original-label

Conversation

architch commented Feb 9, 2019

Copy link
Copy Markdown
Contributor

This PR changes the label to minimum of the vertex ID belonging to a component.
The assigned label of output of Connected Component is currently a random number which is not consistent. If some delta edges are added to the graph, the output label of unaffected components also changes.

Output of friends graph example:

Copy link
Copy Markdown

Codecov Report

Merging #320 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #320      +/-   ##
==========================================
+ Coverage   91.17%   91.23%   +0.05%     
==========================================
  Files          18       18              
  Lines         873      878       +5     
  Branches       73       73              
==========================================
+ Hits          796      801       +5     
  Misses         77       77
Impacted Files Coverage Δ
...cala/org/graphframes/lib/ConnectedComponents.scala 94.69% <100%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89312f3...3dac6e7. Read the comment docs.

Copy link
Copy Markdown
Contributor Author

@mengxr @felixcheung any thoughts on this one.

Copy link
Copy Markdown

This is very helpful, any updates?

followingell commented Nov 25, 2024
edited
Loading

Copy link
Copy Markdown

Hey @rjurney,

Not sure of your status or involvement in this project but is there a chance you could push this through? 🤞

AFAICT this would unlock consistent component IDs across Connected Component executions for components that have unchanged nodes.

rjurney commented Nov 26, 2024
edited
Loading

Copy link
Copy Markdown
Collaborator

@followingell I like this, but my machine is presently in an unhappy state as it relates to this project as I dip my toes in Spark proper waters. What testing have you done?

Copy link
Copy Markdown

@followingell I like this, but my machine is presently in an unhappy state as it relates to this project as I dip my toes in Spark proper waters. What testing have you done?

Unfortunately none. I don't have access at the moment to a machine where I could test this without issues.

rjurney commented Nov 28, 2024

Copy link
Copy Markdown
Collaborator

Okay, give me a few days and I’ll test it.

Copy link
Copy Markdown
Collaborator

We have a lot of pending PRs related to Connected Components. I'm going to push this one forward after merging #552

rjurney commented Mar 31, 2025

Copy link
Copy Markdown
Collaborator

Sounds good. I like this PR.

SemyonSinchenko left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

Copy link
Copy Markdown
Collaborator

I will try to resolve conflicts.

SemyonSinchenko commented Apr 1, 2025
edited
Loading

Copy link
Copy Markdown
Collaborator

@architch Thanks for resolving conflicts. You can use an existing pre-commit hook to fix style (or just run ./build/sbt root/scalafmtAll)

SemyonSinchenko merged commit b18b35f into graphframes:master Apr 1, 2025

Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @architch !

Copy link
Copy Markdown
Collaborator

@architch I want to notify you that after some discussions we made a decision to keep as the default the behavior before your PR: #632

To use min vertex label as component label you will need to explicitly modify the parameter. You can use the config spark.graphframes.useLabelsAsComponents (set it to true) or API (useLabelsAsComponents: bool in Python and setUseLabelsAsComponents(value: Boolean) in JVM).

Sorry about it, but do a breaking change will require too much work from downstream projects. I think for users it won't be hard to set config or argument to true and have a behavior from your PR.

rjurney commented Jul 16, 2025

Copy link
Copy Markdown
Collaborator

This seems like a happy medium :)

Copy link
Copy Markdown
Contributor Author

Thanks @SemyonSinchenko for the info. It makes sense to make it configurable.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL