-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-3974][MLlib] Distributed Block Matrix Abstractions #3200
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
b693209
Ready for Pull request
f378e16
[SPARK-3974] Block Matrix Abstractions ready
aa8f086
[SPARK-3974] Additional comments added
589fbb6
[SPARK-3974] Code review feedback addressed
19c17e8
[SPARK-3974] Changed blockIdRow and blockIdCol
b05aabb
[SPARK-3974] Updated tests to reflect changes
brkyvz 645afbe
[SPARK-3974] Pull latest master
brkyvz 49b9586
[SPARK-3974] Updated testing utils from master
brkyvz d033861
[SPARK-3974] Removed SubMatrixInfo and added constructor without part…
brkyvz 9ae85aa
[SPARK-3974] Made partitioner a variable inside BlockMatrix instead o…
brkyvz ab6cde0
[SPARK-3974] Modifications cleaning code up, making size calculation …
brkyvz ba414d2
[SPARK-3974] fixed frobenius norm
brkyvz 239ab4b
[SPARK-3974] Addressed @jkbradley's comments
brkyvz 1e8bb2a
[SPARK-3974] Change return type of cache and persist
brkyvz 1a63b20
[SPARK-3974] Remove setPartition method. Isn't required
brkyvz eebbdf7
preliminary changes addressing code review
brkyvz f9d664b
updated API and modified partitioning scheme
brkyvz 1694c9e
almost finished addressing comments
brkyvz 140f20e
Merge branch 'master' of github.com:apache/spark into SPARK-3974
brkyvz 5eecd48
fixed gridPartitioner and added tests
brkyvz 24ec7b8
update grid partitioner
mengxr e1d3ee8
minor updates
mengxr feb32a7
update tests
mengxr a8eace2
Merge pull request #2 from mengxr/brkyvz-SPARK-3974
brkyvz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
preliminary changes addressing code review
- Loading branch information
commit eebbdf742e5ef94e9c5f278a07fd94b625117716
There are no files selected for viewing
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
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brkyvz @rezazadeh I'm wondering why you are using (Int,Int) as the block index, as opposed to a class that implements hashCode() to partition the matrix in lexicographic order. Then, the call rdd.partitionBy(new HashPartitioner(numRowBlocks*numColBlocks)) will assure the same partitioning between two distributed matrices. When performing addition or elementwise-multiplication with other distributed matrices, the operation A_ij + B_ij would be local without any shuffle.
Could you please explain the use of (Int,Int) over an Index class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mbhynes. Great question. We did have such a class, it was called
BlockPartition, which later was renamed toSubMatrix. I did try what you suggested, but here was the catch. Even though the partition id's match for partitions, they didn't necessarily end at the same executors. Spark distributes the partitions deterministically, so theoretically, they should end at the same executors, but what happens is, an executor lags, "something" happens, and the partitions then get jumbled around executors. We couldn't (Spark doesn't) guarantee that these partitions end up on the same machine for fault tolerance reasons (it's how the scheduler works). Therefore we needed to have indices as above (which the classSubMatrixhad). To ensure that we add the correct blocks with each other, calling a.joinwas inevitable. Instead of storing the index inside bothSubMatrixand having at as a key, we decided to just index it as above.Then the question becomes: Can users properly index their matrices, properly supply
((Int, Int), Matrix)? To solve this, we will support conversions fromCoordinateMatrixandIndexedRowMatrix, which are convenient storage methods, and have users call.toBlockMatrixfrom these classes.