-
Notifications
You must be signed in to change notification settings - Fork 80
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
Port usage of uninitialized to build_uninit #287
Conversation
let mut a = replicate(&av); | ||
Zip::indexed(&mut a).for_each(|(i, j), elt| { | ||
if i > j { *elt = A::zero() } | ||
}); | ||
a |
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.
This PR has an inefficiency here since zero elements are overwritten after copying (not benchmarked, in some cases probably faster/just as fast because the indexed iterator is slow).
Codecov Report
@@ Coverage Diff @@
## master #287 +/- ##
=======================================
Coverage 89.01% 89.01%
=======================================
Files 71 71
Lines 3577 3578 +1
=======================================
+ Hits 3184 3185 +1
Misses 393 393
Continue to review full report at Codecov.
|
build_uninit is necessary for abstracting over any kind of owned array. The version of take_slice_upper is still inefficient in one way (overwriting after copying), but could likely still be an improvement upon the previous version.
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.
This all looks good to me. The inefficiency in take_slice_upper
is fine for now, IMO. We can always benchmark and improve it later if needed.
I merged this because it looks good to me, we really need to release a new version of |
build_uninit, is necessary for abstracting over any kind of owned array here.
Fixes #277
Requires ndarray 0.15.2