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

WIP: min, max, sum, prod, mean etc on dimensions #2409

Closed
wants to merge 2 commits into from

Conversation

blakejohnson
Copy link
Contributor

First pass at implementing the Dim type from the discussion #2265. For an array A, you can now call, e.g.: max(A, Dim(1)). The Dim can be constructed with integers,
arrays, ranges, or tuples, so all of the following are valid:

Dim(1)
Dim(1, 2)
Dim([1])
Dim([1,2])
Dim([1 2])
Dim(1:2)

I seem to have broken something with BitArrays which I am yet to figure out.

Still need to modify cumsum and cumprod. All the stats functions need similar updates.

First pass at implemented the Dim type for JuliaLang#2265. For an array A, you can now
call, e.g.: max(A, Dim(1)). The Dim type can be constructed with integers,
arrays, ranges, or tuples, so all of the following are valid:
Dim(1)
Dim(1, 2)
Dim([1])
Dim([1,2])
Dim([1 2])
Dim(1:2)
@pao
Copy link
Member

pao commented Feb 26, 2013

Travis failures don't appear to be your fault. Filing new issues for them.

@stevengj
Copy link
Member

Dim should take any iterable collection of integers, which could be implemented via:

Dim(d) = Dim(Int[i for i in d])

with no need for special cases for Range, Tuple, Int, etcetera.

But I'm not completely sold on the whole Dim syntax in general.

@blakejohnson
Copy link
Contributor Author

@stevengj How do I get that to work while also supporting Dim(1, 2, 3...)?

@stevengj
Copy link
Member

Dim(arg1, arg2, args...) = Dim([arg1,arg2,args...])

@blakejohnson
Copy link
Contributor Author

@stevengj Apparently a Range is an AbstractVector. So, if I make just these two outer constructors:

Dim(d) = Dim(Int[d[i] for i=1:length(d)])
Dim(d1, d2...) = Dim([d1, d2...])

then a call to Dim(1:3) doesn't vectorize the range.

@kmsquire
Copy link
Member

It probably shouldn't really matter if range isn't vectorized, e.g.,

julia> z = zeros(2,2,2,2); z[:]=1:16; sum(z, 1:4)
1x1x1x1 Float64 Array:
[:, :, 1, 1] =
 136.0

@stevengj
Copy link
Member

Why waste storage by converting a range into an explicit vector of elements?

@pao
Copy link
Member

pao commented Feb 27, 2013

On further review, #2411 might actually be caused by this, so please look into that if we're going to pursue this further.

@blakejohnson
Copy link
Contributor Author

@pao yes, I do think the BitArray issues are caused by my changes. However, the winds seem to shifting away from this approach over in #2265. So, I am going to hold off on further development until that issue is settled.

@pao
Copy link
Member

pao commented Feb 27, 2013

Fair enough. I'll go ahead and close #2411 to get it off the radar, then.

@StefanKarpinski
Copy link
Member

I have to say, I really don't care for the Dim type approach. This is a solid implementation of it, but it really rubs me the wrong way. I think we should just be draconian about the second argument of functions that accept dimension specifications being the dimension and introduce alternate function names for versions that do something else, e.g. varm(m,v,[d]) and stdm(m,v,[d]) for variance and standard deviation calculations with known means.

@blakejohnson
Copy link
Contributor Author

@StefanKarpinski I'm with you on the varm, stdm, etc... I'll have a go at implementing it and open a new pull request.

@johnmyleswhite
Copy link
Member

I also really like the varm, stdm approach. I basically never need those functions.

@StefanKarpinski
Copy link
Member

"Varm" also means "warm" in Swedish, which gives me an, er, warm and fuzzy feeling.

@toivoh
Copy link
Contributor

toivoh commented Feb 27, 2013

@StefanKarpinski: Yeah, that felt pretty funny to me too. Do you know Swedish?

@StefanKarpinski
Copy link
Member

I do – my mother's from Sweden and it was my first language.

@toivoh
Copy link
Contributor

toivoh commented Feb 27, 2013

Aha, där ser man. Fast det är nog bättre att vi pratar engelska här ändå.

@StefanKarpinski
Copy link
Member

Kanske vi skulle översätta dokumentationen til svenska? Nä... alla svenskar föstår engelska ändå.

@toivoh
Copy link
Contributor

toivoh commented Feb 27, 2013

Precis :)

@blakejohnson blakejohnson deleted the dimension-args branch February 27, 2013 20:14
Also shelves stats changes for later, and simplifies Dim constructors.
@blakejohnson blakejohnson restored the dimension-args branch March 1, 2013 03:53
@blakejohnson
Copy link
Contributor Author

Since the max/min solution is still being discussed, I am re-opening this pull request. I've fixed the test failures. If we decide to continue with this approach, I could use a suggestion for a less hacky means of comparing the regions in suitespare.jl's reducedim function.

@blakejohnson blakejohnson reopened this Mar 1, 2013
@blakejohnson
Copy link
Contributor Author

Closing, as stdm and varm are implemented in #2561, and it seems we are waiting on keyword args to fix max(v, (), dim).

@blakejohnson blakejohnson deleted the dimension-args branch August 23, 2014 03:03
staticfloat added a commit that referenced this pull request Mar 6, 2021
This should include the recent `is_stdlib()` fixes.  Short commit log:

```
7a9d9654 (HEAD -> master, origin/master, origin/HEAD) [ext/HSG]: Store next release _and_ latest nightly (#2418)
7b870924 [ext/HSG]: Enable generating historical stdlibs on macOS (#2417)
5d496193 Update Project.toml
feada149 only use the stdlib version cache if a custom version is given to the resolver
bae808dc Fix Markdown table formatting (#2416)
6e8b6214 Update docstrings for io kwargs, some io kwarg fixes, update stdlib list (#2402)
c2e3879e Mark the "STDLIBS_BY_VERSION up-to-date" test as broken (#2409)
```
staticfloat added a commit that referenced this pull request Mar 6, 2021
This should include the recent `is_stdlib()` fixes.  Short commit log:

```
7a9d9654 (HEAD -> master, origin/master, origin/HEAD) [ext/HSG]: Store next release _and_ latest nightly (#2418)
7b870924 [ext/HSG]: Enable generating historical stdlibs on macOS (#2417)
5d496193 Update Project.toml
feada149 only use the stdlib version cache if a custom version is given to the resolver
bae808dc Fix Markdown table formatting (#2416)
6e8b6214 Update docstrings for io kwargs, some io kwarg fixes, update stdlib list (#2402)
c2e3879e Mark the "STDLIBS_BY_VERSION up-to-date" test as broken (#2409)
```
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
This should include the recent `is_stdlib()` fixes.  Short commit log:

```
7a9d9654 (HEAD -> master, origin/master, origin/HEAD) [ext/HSG]: Store next release _and_ latest nightly (JuliaLang#2418)
7b870924 [ext/HSG]: Enable generating historical stdlibs on macOS (JuliaLang#2417)
5d496193 Update Project.toml
feada149 only use the stdlib version cache if a custom version is given to the resolver
bae808dc Fix Markdown table formatting (JuliaLang#2416)
6e8b6214 Update docstrings for io kwargs, some io kwarg fixes, update stdlib list (JuliaLang#2402)
c2e3879e Mark the "STDLIBS_BY_VERSION up-to-date" test as broken (JuliaLang#2409)
```
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
This should include the recent `is_stdlib()` fixes.  Short commit log:

```
7a9d9654 (HEAD -> master, origin/master, origin/HEAD) [ext/HSG]: Store next release _and_ latest nightly (JuliaLang#2418)
7b870924 [ext/HSG]: Enable generating historical stdlibs on macOS (JuliaLang#2417)
5d496193 Update Project.toml
feada149 only use the stdlib version cache if a custom version is given to the resolver
bae808dc Fix Markdown table formatting (JuliaLang#2416)
6e8b6214 Update docstrings for io kwargs, some io kwarg fixes, update stdlib list (JuliaLang#2402)
c2e3879e Mark the "STDLIBS_BY_VERSION up-to-date" test as broken (JuliaLang#2409)
```
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.

7 participants