Skip to content

[SPARK-15784][ML]:Add Power Iteration Clustering to spark.ml #15770

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.

Sign up for GitHub

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

Closed
wangmiao1981 wants to merge 28 commits into from
Closed

[SPARK-15784][ML]:Add Power Iteration Clustering to spark.ml #15770

Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b80bb1f
add pic framework (model, class etc)
wangmiao1981 Jun 13, 2016
75004e8
change a comment
wangmiao1981 Jun 13, 2016
e1d9a33
add missing functions fit predict load save etc.
wangmiao1981 Jun 17, 2016
f8343e0
add unit test flie
wangmiao1981 Jun 18, 2016
c62a2c0
add test cases part 1
wangmiao1981 Jun 20, 2016
1277f75
add unit test part 2: test fit, parameters etc.
wangmiao1981 Jun 20, 2016
f50873d
fix a type issue
wangmiao1981 Jun 20, 2016
88a9ae0
add more unit tests
wangmiao1981 Jun 21, 2016
0618815
delete unused import and add comments
wangmiao1981 Jun 21, 2016
04fddbd
change version to 2.1.0
wangmiao1981 Oct 25, 2016
b49f4c7
change PIC as a Transformer
wangmiao1981 Nov 3, 2016
d3f86d0
add LabelCol
wangmiao1981 Nov 4, 2016
655bc67
change col implementation
wangmiao1981 Nov 4, 2016
d5975bc
address some of the comments
wangmiao1981 Feb 17, 2017
f012624
add additional test with dataset having more data
wangmiao1981 Feb 21, 2017
bef0594
change input data format
wangmiao1981 Mar 14, 2017
a4bee89
resolve warnings
wangmiao1981 Mar 15, 2017
0f97907
add neighbor and weight cols
wangmiao1981 Mar 16, 2017
015383a
address review comments 1
wangmiao1981 Aug 15, 2017
2d29570
fix style
wangmiao1981 Aug 15, 2017
af549e8
remove unused comments
wangmiao1981 Aug 15, 2017
9b4f3d5
add Since
wangmiao1981 Aug 15, 2017
e35fe54
fix missing >
wangmiao1981 Aug 17, 2017
73485d8
fix doc
wangmiao1981 Aug 17, 2017
bd5ca5d
Merge github.com:apache/spark into pic
wangmiao1981 Sep 12, 2017
3b0f71c
Merge github.com:apache/spark into pic
wangmiao1981 Oct 25, 2017
752b685
address review comments
wangmiao1981 Oct 25, 2017
cfa18af
fix unit test
wangmiao1981 Oct 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove unused comments
  • Loading branch information
@wangmiao1981
wangmiao1981 committed Aug 16, 2017
commit af549e8dca3cd6488c3217e2eb9fe61025b11cd7
11 changes: 0 additions & 11 deletions mllib/src/main/scala/org/apache/spark/ml/clustering/PowerIterationClustering.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,24 +170,13 @@ class PowerIterationClustering private[clustering] (
@Since("2.3.0")
override def transform(dataset: Dataset[_]): DataFrame = {
val sparkSession = dataset.sparkSession
/*
val rdd: RDD[(Long, Long, Double)] =
dataset.select(col($(idCol)), col($(neighborCol)), col($(weightCol))).rdd.map {
case Row(id: Long, nbr: Vector, weight: Vector) => (id, nbr, weight)
}.flatMap{ case (id, nbr, weight) =>
require(nbr.size == weight.size,
"The length of neighbor list must be equal to the the length of the weight list.")
val ids = Array.fill(nbr.size)(id)
ids.zip(nbr.toArray).zip(weight.toArray)}.map {case ((i, j), k) => (i, j.toLong, k)}
*/
val rdd: RDD[(Long, Long, Double)] =
dataset.select(col($(idCol)), col($(neighborCol)), col($(weightCol))).rdd.flatMap {
case Row(id: Long, nbr: Vector, weight: Vector) =>
Copy link
Contributor

@WeichenXu123 WeichenXu123 Sep 8, 2017

Choose a reason for hiding this comment

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

The PIC require input graph matrix to be symmetric, and the weight should be non-negative. It is better to check them here. But checking symmetric seems cost too much, I have no good idea for now. cc @jkbradley Do you have some thoughts ?

Copy link
Contributor Author

@wangmiao1981 wangmiao1981 Oct 5, 2017

Choose a reason for hiding this comment

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

I think checking symmetric is too much for PIC in this data format. Maybe, we can omit the check and put a comment and INFO on console to let users take care of it. @WeichenXu123

Copy link
Contributor

@WeichenXu123 WeichenXu123 Oct 31, 2017

Choose a reason for hiding this comment

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

OK I agree.

Copy link
Member

@jkbradley jkbradley Apr 3, 2018

Choose a reason for hiding this comment

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

I agree about not checking for symmetry as long as we document it.

But I do have one suggestion: Let's take neighbors and weights as Arrays, not Vectors. That may help prevent users from mistakenly passing in feature Vectors.

require(nbr.size == weight.size,
"The length of neighbor list must be equal to the the length of the weight list.")
val ids = Array.fill(nbr.size)(id)
for (i <- 0 until ids.size) yield (ids(i), nbr(i).toLong, weight(i))}
Copy link
Contributor

@WeichenXu123 WeichenXu123 Sep 8, 2017

Choose a reason for hiding this comment

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

This line code do not performs well, why you expand the var id into a array filled with the same value ?
You can use:

nbr.toArray.toIterator.zip(weight.toArray.toIterator).map(x => (id, x._1.toLong, x._2.toLong))

Copy link
Contributor Author

@wangmiao1981 wangmiao1981 Oct 5, 2017

Choose a reason for hiding this comment

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

I remember last time you mentioned not using zip. I might get it wrong. By the way, it seems we need a helper function, do you want a helper function takes GraphFrame and returns the RDD used by PIC?

Copy link
Contributor

@WeichenXu123 WeichenXu123 Oct 9, 2017

Choose a reason for hiding this comment

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

Ah, I don't like Array.zip because it generate a temp array But Array.toIterator.zip do not have this problem, it is iterator zip.


val algorithm = new MLlibPowerIterationClustering()
.setK($(k))
.setInitializationMode($(initMode))
Expand Down