Skip to content

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

Merged
quinnj merged 2 commits into
JuliaData:mainfrom
laborg:fixmap!
Sep 29, 2026
Merged

quinnj merged 2 commits 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.

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (fa840f9) to head (8ec399d).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
+ Coverage   94.82%   95.60%   +0.77%     
==========================================
  Files           5        5              
  Lines        1043     1046       +3     
==========================================
+ Hits          989     1000      +11     
+ Misses         54       46       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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.

Thanks @laborg, and sorry this sat so long. Verified: iterating zip of both index sets stops at the shorter source, which matches Base's map! contract and fixes #115. Full suite passes with this merged on top of current main.

@quinnj
quinnj merged commit 7752d36 into JuliaData:main Sep 29, 2026
6 checks passed
@quinnj quinnj mentioned this pull request Sep 29, 2026
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