Vectorize DotFeatures covariance/mean calculation #3908
Merged
Conversation
This file contains 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
|
Maybe offer a flag for that? And so some clever auto settings of it.... |
Codecov Report
@@ Coverage Diff @@
## develop #3908 +/- ##
===========================================
- Coverage 53.29% 53.24% -0.05%
===========================================
Files 1432 1432
Lines 104428 104422 -6
===========================================
- Hits 55652 55597 -55
- Misses 48776 48825 +49
Continue to review full report at Codecov.
|
|
I'd be keen to have a flag that improves speed substantially here, trading off with storing the dataset twice. Most of the time that is fine anyways. Thoughts? |
|
Yeah, seems reasonable, what about auto setting you mentioned? any
suggestion?
Il 09/lug/2017 17:35, "Heiko Strathmann" <notifications@github.com> ha
scritto:
… I'd be keen to have a flag that improves speed substantially here, trading
off with storing the dataset twice. Most of the time that is fine anyways.
Thoughts?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3908 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKH6Tt1ReE75OjJmDe_ZP2T96Q-ERwHvks5sMPM1gaJpZM4OPre1>
.
|
|
For now, I would just offer a flag "copy_data_for_speed" which would be appropriately documented, and enabled by default |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Would be nice also to parallelize the covariance main loop but is not obvious how to do it properly since it works on one vector at a time, tried a few variants with openmp but it didn’t improve; the straightforward way to speed-up get_cov would be to compute it by matrix prod (tried, approx. 4X on 4 threads), but this requires the storage of the centered data matrix while the current algorithm doesn’t need that.