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

More Cartesian housekeeping, plus implementation of copy! #5671

Merged
merged 7 commits into from
Feb 8, 2014

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Feb 4, 2014

  • Cleans up @ngenerate to add return-type annotation
  • Adds @nsplat to generate specialized versions of functions that avoid splatting
  • Adds quite a few new tests of type inference
  • Implements cartesian copy! for faster performance on SubArrays

The addition of @nsplat is not strictly necessary; I could ditch that and write them out by hand, if desired. The main motivation for adding it was to automatically synchronize the use of splatting with changes in CARTESIAN_DIMS.

Performance gain on copy! is the usual:
Master:

julia> A = rand(1000,1000,50);

julia> B = zeros(1000,1000,50);

julia> S = sub(B, :, :, :);

julia> @time copy!(S, A);
elapsed time: 2.144633973 seconds (48 bytes allocated)

This branch:

julia> @time copy!(S, A);
elapsed time: 0.399700238 seconds (48 bytes allocated)

CC @carlobaldassi.

@carlobaldassi
Copy link
Member

Good! I'll rebase my branch as soon as this is in.

I thought this change would have allowed getting rid of things like these:

checksize(A, I) = (_checksize(A, I); return nothing)
checksize(A, I, J) = (_checksize(A, I, J); return nothing)
checksize(A, I...) = (_checksize(A, I...); return nothing)

@timholy
Copy link
Sponsor Member Author

timholy commented Feb 4, 2014

It does, I just missed them---they were just hard to see, since they were in the top 6 lines of the file 😄. Thanks! (If you see any more, don't hesitate to ask, or to fix when you submit your changes.)

Fixed and rebased. Hmm, the original commits are still showing up on this page, but my git history for this branch doesn't show them---anyone know what's going on? Does github now keep copies of old commits when you do a forced update?

@timholy
Copy link
Sponsor Member Author

timholy commented Feb 4, 2014

Ah, now magically github seems to have gotten this straightened out.

timholy added a commit that referenced this pull request Feb 8, 2014
More Cartesian housekeeping, plus implementation of copy!
@timholy timholy merged commit 836f0ae into master Feb 8, 2014
@timholy
Copy link
Sponsor Member Author

timholy commented Feb 8, 2014

All yours, @carlobaldassi!

@timholy timholy deleted the teh/cartesian branch February 13, 2014 12:31
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.

None yet

2 participants