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

base: make diff() use views and broadcasting #29827

Merged
merged 2 commits into from
Oct 31, 2018

Conversation

stev47
Copy link
Contributor

@stev47 stev47 commented Oct 27, 2018

fixes #27957
fixes #15414 (didn't see this at the time of coding)

This uses array views and broadcasting to implement diff() in arbitrary dimension.
Added benefit is a significant performance increase for arrays with more than about 10 elements (at least in my tests).

x = rand(1000)
y = rand(1000,1000)
z = rand(10)
@btime diff($x);
@btime diff($y, dims=2);
@btime diff($z);

before:

919.474 ns (3 allocations: 7.98 KiB)
1.612 ms (4 allocations: 7.62 MiB)
41.718 ns (3 allocations: 208 bytes)

after:

504.201 ns (3 allocations: 8.03 KiB)
864.279 μs (4 allocations: 7.62 MiB)
41.598 ns (3 allocations: 256 bytes)

base/multidimensional.jl Outdated Show resolved Hide resolved
base/multidimensional.jl Outdated Show resolved Hide resolved
base/multidimensional.jl Outdated Show resolved Hide resolved
Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

Very nice @stev47, just one tiny thing to fix and then I think this can be merged. There's some chance we'd need #28941 to solve this elegantly for arrays with offset axes, so I think it's fine to restrict this to 1-indexed arrays for now.

(@juliohm, this is the implementation I was hinting at with my comment about "in one go.")

base/multidimensional.jl Outdated Show resolved Hide resolved
@juliohm
Copy link
Contributor

juliohm commented Oct 28, 2018

Thanks @stev47 for the addition, and thanks @timholy for pinging. 👍

@timholy
Copy link
Member

timholy commented Oct 29, 2018

Sorry for one more request, but I think the docstring needs updating too. Does this need a NEWS item?

@KristofferC KristofferC added the needs news A NEWS entry is required for this change label Oct 29, 2018
@stev47
Copy link
Contributor Author

stev47 commented Oct 29, 2018

updated the docstring

@mbauman mbauman removed the needs news A NEWS entry is required for this change label Oct 30, 2018
@timholy timholy merged commit eabc5de into JuliaLang:master Oct 31, 2018
fredrikekre added a commit that referenced this pull request Dec 5, 2018
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
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.

diff(Vector,dims=1) Extend diff() to arbitrary dimensions
7 participants