initialize Arrays with great dignity #24652

Sacha0 · 2017-11-19T17:58:44Z

This pull request replaces the existing primitive Array constructors with equivalents requiring uninitialized as their first argument, e.g. Array{T,N}(d::VarArg{Int,N}) -> Array{T,N}(uninitialized, d::VarArg{Int,N}), and reimplements the existing constructors in terms of those new equivalents.

This pull request is the equivalent of #24400 with uninitialized in place of junk, and is the first 1.0-necessary step for #24595.

Question: uninitialized is presently a methodless function (as with junk in #24400), which makes sense only if we plan to simultaneously introduce a convenience constructor uninitialized(T, shape...). But if we do not introduce such a convenience constructor, then perhaps uninitialized should instead be a type. IIRC, whether we should introduce such a convenience constructor hasn't been explicitly decided yet. Thoughts? Thanks and best!

jebej · 2017-11-19T18:18:29Z

I'd be happy to have uninitialized as a function, if only to replace the deprecated Array{T,N}(d) constructors.

Jutho · 2017-11-19T21:06:24Z

If uninitialized(T, shape...) creates something (probably an Array), is this not another member of the family of functions (zeros, ones, rand) that #24595 seeks to deprecate, because of all the reasons given there?

kmsquire · 2017-11-19T22:07:15Z

[WIP] initialize Arrays with great dignity

@Sacha0, I love your attitude and sense of humor!

Sacha0 · 2017-11-19T23:31:10Z

If uninitialized(T, shape...) creates something (probably an Array), is this not another member of the family of functions (zeros, ones, rand) that #24595 seeks to deprecate, because of all the reasons given there?

To clarify, the analysis in #24595 does not support deprecating all convenience constructors. Rather, that analysis suggests that certain existing convenience constructors make a poor foundation on which to build more general array constructors, and that the set of existing Array constructors should perhaps be pruned to a sounder subset :). Best!

Sacha0 · 2017-11-19T23:34:01Z

Much thanks for the kind words @kmsquire!! :)

mschauer · 2017-11-20T08:44:42Z

What is planned for sprand? This is a bit special as it cannot be realised as dispatch on a function name and has an additional argument. Sorting that out might help with the >Question.

Sacha0 · 2017-11-20T17:29:21Z

What is planned for sprand? This is a bit special as it cannot be realised as dispatch on a function name and has an additional argument. Sorting that out might help with the >Question.

sprand is indeed tricky and its fate remains to be decided; fortunately that decision is not pressing, what with things sparse moving to stdlib :). More generally, whether and if so how to evolve the *rand* family of convenience constructors is somewhat orthogonal to the concerns here; #24595 is likely the best place for that discussion. Thanks and best! :)

mschauer · 2017-11-20T18:51:08Z

A miscommunication: I don't want to discuss sprand. You asked whether uninitialized should be a function and I just asked if you have thought of sprand (because having that in mind helps imho to make a good decision here.)

ararslan · 2017-11-20T21:05:54Z

FWIW I prefer uninitialized(T, dims) to Array{T}(uninitialized, dims), if that's still up for discussion.

Sacha0 · 2017-11-20T21:07:10Z

On the subject of whether to introduce an uninitialized(T, shape...) convenience constructor, triage was largely ambivalent and so concluded favoring the conservative option: Make uninitialized a type for now and introduce an uninitialized convenience constructor later if desired. Best!

mschauer · 2017-11-20T21:11:52Z

Make uninitialized a type

Do you mean "type" or "instance of a type"? Latest developments seem to favour dispatch on instances (e.g. Val{true} versus Val(true)). (Edit: Thanks.)

Jutho · 2017-11-20T21:16:39Z

That's true (though quoted slightly incorrect: it's Val{true} which is of type ::Type{Val{true} versus Val(true) which actually creates Val{true}() using a @pure constructor and is of type ::Val{true}).

For consistency (capitalization of types), I would also favour a singleton type with a single instance

struct Uninitialized end
const uninitialized = Uninitialized()

function Array{T,N}(::Uninitialized, ...)

which can then be called as Array{T,N}(uninitialized, dims).

Sacha0 · 2017-11-20T21:17:07Z

FWIW I prefer uninitialized(T, dims) to Array{T}(uninitialized, dims), if that's still up for discussion.

To clarify, we need Array[{...}](uninitialized, shape...) in any case, that being the realization for Arrays of the general model MyArray[{...}](uninitialized[, otherspecs...]). The question is whether we should also introduce the convenience constructor uninitialized(T, shape...) for Arrays at this time :).

Sacha0 · 2017-11-20T21:19:48Z

Rebased and updated with the implementation @Jutho suggested above. Best!

Sacha0 · 2017-11-21T04:46:29Z

The latest iteration appears to be in shape. Absent objections or requests for time, I plan to merge these changes tomorrow evening (Tuesday evening PT) or later. Best!

fredrikekre

Looks good, but tests and docs would be nice :)

fredrikekre · 2017-11-21T07:13:55Z

base/boot.jl

@@ -123,7 +123,7 @@ export
    SimpleVector, AbstractArray, DenseArray, NamedTuple,
    # special objects
    Function, CodeInfo, Method, MethodTable, TypeMapEntry, TypeMapLevel,
-    Module, Symbol, Task, Array, WeakRef, VecElement,
+    Module, Symbol, Task, Array, Uninitialized, uninitialized, WeakRef, VecElement,


Should Uninitialized be exported? It does not seem necessary for the usage

Uninitialized must be exported, lest Julia not build :).

Why is that?

Uninitialized/uninitialized must be visible everywhere uninitialized arrays are or will be constructed; these exports make Uninitialized/uninitialized visible beyond Core. The build failure absent these exports comes from the definitions using Uninitialized/uninitialized in sysimg.jl. Does that clarify? :)

Though wouldn't it be possible to export only uninitialized from Base (not Core)? Should Uninitialized be used outside of Base?

Uninitialized should see use outside Base, yes, in definitions of constructors for uninitialized arrays generally (see #24595). Additionally uninitialized is merely an alias for Uninitialized(), and hence uninitialized cannot do its magic without Uninitialized visible. Does that clarify? :)

fredrikekre · 2017-11-21T07:14:39Z

base/boot.jl

-Array{T,2}(m::Int, n::Int) where {T} =
+## primitive Array constructors
+struct Uninitialized end
+const uninitialized = Uninitialized()


This could perhaps use a docstring -- I think uninitialized will be something people will look up in the manual when it goes live.

Good call! I will add docstrings shortly :).

Docstrings live. Thanks! :)

Sacha0 · 2017-11-21T16:23:40Z

tests [...] would be nice :)

By nature of underlying every Array allocation, these methods are thoroughly tested as it stands :). When deprecating Array(shape...)-like calls, I will explicitly migrate the relevant tests to their Array(uninitialized, shape...)-like equivalents as well. Best!

fredrikekre · 2017-11-21T16:51:53Z

By nature of underlying every Array allocation, these methods are thoroughly tested as it stands :). When deprecating Array(shape...)-like calls, I will explicitly migrate the relevant tests to their Array(uninitialized, shape...)-like equivalents as well. Best!

Makes sense. Also, sorry for beeing a pain, but do you mind adding the docstrings todoc/src/stdlib/... 😄

Sacha0 · 2017-11-21T17:00:24Z

Also, sorry for beeing a pain, but do you mind adding the docstrings todoc/src/stdlib/... 😄

Not at all a pain! To the contrary, I much appreciate your thorough and rapid reviews, and comments like these are super helpful! Revised accordingly, and thanks again! :)

…initialized, dims...) etc.

Sacha0 · 2017-11-22T17:12:55Z

Thanks all! :)

This constructor overwrites what was added in #24652. As long as memory is uninitialized (rather than zeroed) by default, these constructors do the same thing. Removing this constructor fixes a method overwritten warning during the build.

Sacha0 added arrays [a, r, r, a, y, s] needs decision A decision on this change is needed triage This should be discussed on a triage call labels Nov 19, 2017

Sacha0 removed the triage This should be discussed on a triage call label Nov 20, 2017

Sacha0 force-pushed the initdig branch from e944ddd to 965b289 Compare November 20, 2017 21:12

Sacha0 force-pushed the initdig branch from 965b289 to 42e1c54 Compare November 20, 2017 21:19

Sacha0 force-pushed the initdig branch from 42e1c54 to 78268c8 Compare November 21, 2017 02:47

Sacha0 changed the title ~~[WIP] initialize Arrays with great dignity~~ initialize Arrays with great dignity Nov 21, 2017

Sacha0 mentioned this pull request Nov 21, 2017

[WIP] add some junk #24400

Closed

fredrikekre reviewed Nov 21, 2017

View reviewed changes

Sacha0 force-pushed the initdig branch from 78268c8 to ec152be Compare November 21, 2017 16:42

Sacha0 force-pushed the initdig branch from ec152be to fc3e1cf Compare November 21, 2017 16:58

fredrikekre approved these changes Nov 21, 2017

View reviewed changes

Replace primitive Array{...}(dims...) constructors with Array{...}(un…

353da20

…initialized, dims...) etc.

Sacha0 force-pushed the initdig branch from fc3e1cf to 353da20 Compare November 21, 2017 23:10

Sacha0 merged commit 5dcb44a into JuliaLang:master Nov 22, 2017

Sacha0 deleted the initdig branch November 22, 2017 17:12

ararslan mentioned this pull request Nov 22, 2017

Remove overwriting empty vector constructor #24710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initialize Arrays with great dignity #24652

initialize Arrays with great dignity #24652

Sacha0 commented Nov 19, 2017

jebej commented Nov 19, 2017 •

edited

Loading

Jutho commented Nov 19, 2017

kmsquire commented Nov 19, 2017

Sacha0 commented Nov 19, 2017

Sacha0 commented Nov 19, 2017

mschauer commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

mschauer commented Nov 20, 2017

ararslan commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

mschauer commented Nov 20, 2017 •

edited

Loading

Jutho commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

Sacha0 commented Nov 21, 2017

fredrikekre left a comment

fredrikekre Nov 21, 2017

Sacha0 Nov 21, 2017

nalimilan Nov 22, 2017

Sacha0 Nov 22, 2017

nalimilan Nov 22, 2017

Sacha0 Nov 22, 2017

fredrikekre Nov 21, 2017

Sacha0 Nov 21, 2017

Sacha0 Nov 21, 2017

Sacha0 commented Nov 21, 2017

fredrikekre commented Nov 21, 2017

Sacha0 commented Nov 21, 2017

Sacha0 commented Nov 22, 2017

initialize Arrays with great dignity #24652

initialize Arrays with great dignity #24652

Conversation

Sacha0 commented Nov 19, 2017

jebej commented Nov 19, 2017 • edited Loading

Jutho commented Nov 19, 2017

kmsquire commented Nov 19, 2017

Sacha0 commented Nov 19, 2017

Sacha0 commented Nov 19, 2017

mschauer commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

mschauer commented Nov 20, 2017

ararslan commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

mschauer commented Nov 20, 2017 • edited Loading

Jutho commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

Sacha0 commented Nov 20, 2017

Sacha0 commented Nov 21, 2017

fredrikekre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sacha0 commented Nov 21, 2017

fredrikekre commented Nov 21, 2017

Sacha0 commented Nov 21, 2017

Sacha0 commented Nov 22, 2017

jebej commented Nov 19, 2017 •

edited

Loading

mschauer commented Nov 20, 2017 •

edited

Loading