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

Using new Cartesian tools for copy! #5411

Closed
lindahua opened this issue Jan 15, 2014 · 7 comments
Closed

Using new Cartesian tools for copy! #5411

lindahua opened this issue Jan 15, 2014 · 7 comments
Labels
performance Must go faster

Comments

@lindahua
Copy link
Contributor

We should reimplement the copy! function for abstract arrays using @timholy's new Cartesian tools.

@timholy
Copy link
Sponsor Member

timholy commented Jan 15, 2014

I'm fine with that, and I also understand if people have reservations about Cartesian invading base too deeply. Since it's my baby I probably don't have the best perspective on this, so I will defer to the wisdom of others.

@StefanKarpinski
Copy link
Sponsor Member

Paradoxically, I want Cartesian to invade base because I want it to go away. To explain that a bit further, what I'd like is for this Cartesian code to serve to help us understand exactly what we need from language features to replace all of it.

@timholy
Copy link
Sponsor Member

timholy commented Jan 15, 2014

That's a very reasonable thought. However, the reality is probably that many of the remaining applications in abstractarray are "trivial," and so won't teach us much more. reducedim, broadcast, and getindex illustrate more of its power. I haven't thought yet about a possible extension for setindex!, that might be very informative.

I'm happy do all the conversions myself, as anything "easy" would not take me long. But if others want to tackle some of these as an exercise in learning more about how it all works---and therefore what we need to do to ditch Cartesian---I'm happy to let you have at it.

@StefanKarpinski
Copy link
Sponsor Member

No, feel free. You can probably shave another 500 lines off of Base ;-)

@JeffBezanson
Copy link
Sponsor Member

At this point we should only use it where there are performance problems (e.g. SubArray) and/or where the existing code is ugly anyway.

@StefanKarpinski
Copy link
Sponsor Member

Or where we can avoid using linear indexing and thereby make the algorithm more generic.

timholy added a commit that referenced this issue Feb 4, 2014
timholy added a commit that referenced this issue Feb 4, 2014
@timholy
Copy link
Sponsor Member

timholy commented Feb 4, 2014

Implemented in #5671.

@timholy timholy closed this as completed Feb 4, 2014
timholy added a commit that referenced this issue Feb 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

No branches or pull requests

4 participants