Skip to content

fix map!(f,dest::ChainedVector,src::AbstractVector) - #116

Open
laborg wants to merge 1 commit into
JuliaData:mainfrom
laborg:fixmap!
Open

laborg wants to merge 1 commit into
JuliaData:mainfrom
laborg:fixmap!

Conversation

@laborg

@laborg laborg commented Dec 23, 2024

Copy link
Copy Markdown
Contributor

Fixes #115

With this PR the src is iterated instead of the probably longer destination.

Performance is better on this PR:

a,b=ChainedVector([rand(Int,5) for _ in 1:20000]),[1:100000...] # many chunks
@btime map!(i->-i,a,b)
# main: 188us
# pr: 153us

a,b=ChainedVector([[1:100000...]]),[1:100000...] # one chunk
@btime map!(i->-i,a,b)
# main: 171us
# pr: 73.8us

the longer `dest` was iterated, leading to an error if `src` was shorter than `dest`.

@quinnj quinnj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Reviewed by AI] Reviewed d179757d1480b996c56019e435be736058d0f5ed. No actionable findings in the changed overloads. The full suite passed locally on Julia 1.13/macOS arm64. Focused Base-vector checks cover empty and shorter sources, unchanged destination tails, chunk boundaries, and overlapping views. The short-source example fails on the base and passes here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

map!(f,dest::ChainedVector,src::AbstractArray) doesn't work for shorter src

2 participants