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

MAHOUT-1837: Fixed dense bug in drm/package.blockify() #252

Closed

Conversation

andrewpalumbo
Copy link
Member

Create a SparseRowMatrix by default in order to keep OOM errors from occurring in blockify() per conversation in: 727e5be#commitcomment-18771603. run densityAnalysis() on that and convert to dense if requirements are met.

@dlyubimov
Copy link
Contributor

Looks good to me

} else {
new SparseRowMatrix(vectors.length, blockncol, vectors, true, false)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should the default here be a SparseRowMatrix of Random Access Sparse Vectors? Seems so. I.e.
This line should probably read:

new SparseRowMatrix(vectors.length, blockncol, vectors, true, true)

rather than:

new SparseRowMatrix(vectors.length, blockncol, vectors, true, false)

as is, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, i believe sequential should stay as it will be more natural (ordered)
when converted to CSR which hopefuly will be our most common modus operandi
for exponential algorithms.

in fact, random access rarely makes sense at all for block-wise algorithms.

On Fri, Aug 26, 2016 at 8:03 PM, Andrew Palumbo notifications@github.com
wrote:

In spark/src/main/scala/org/apache/mahout/sparkbindings/drm/package.scala
#252 (comment):

     } else {
  •      new SparseRowMatrix(vectors.length, blockncol, vectors, true, false)
    

Should the default here be a SparseRowMatrix of Random Access Sparse
Vectors? Seems so. I.e.
This line should probably read:

new SparseRowMatrix(vectors.length, blockncol, vectors, true, true)

rather than:

new SparseRowMatrix(vectors.length, blockncol, vectors, true, false)

as is, correct?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/apache/mahout/pull/252/files/5d2f5cc9746f968d0c776f869070dd9a439de9f1#r76509011,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAf7_zKcdpkHa0zDQnNvOFMuXQHNW897ks5qj6j4gaJpZM4JtpfO
.

Copy link
Member Author

Choose a reason for hiding this comment

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

thx.

@asfgit asfgit closed this in f4a71d0 Aug 27, 2016
@andrewpalumbo
Copy link
Member Author

Thanks again @AddictedCS for the bug report.

@AddictedCS
Copy link

Thanks @andrewpalumbo for a quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants