rewrite all calls to ones(...) #25087

fredrikekre · 2017-12-14T23:22:33Z

Some joint work between me and @Sacha0. I have to say, rewriting all of the ones call to fill is a real pleasure. I was skeptic at first, but now I love fill just as much as @Sacha0.

Please read JuliaLang/LinearAlgebra.jl#484 and #24595 and the linked discussion therein.

This PR has also been rebased for hours through the large changes of eye, and uninitialized array constructors, so it better get merged! 😄

Sacha0 · 2017-12-14T23:46:59Z

I was skeptic at first, but now I love fill just as much as @Sacha0.

Oh gosh 😂

Sacha0 · 2017-12-14T23:48:32Z

Thanks for picking this work up @fredrikekre! :) Please feel welcome to squash out / remove / * my early commits as you see fit.

Sacha0 · 2017-12-18T20:27:30Z

NEWS.md

    and/or shape depending on `opts...`. For an algebraic multiplicative identity,
    consider `one(A)` ([#24656]).

+  * `ones(dims...)` `and ones(T, dims)` have been deprecated. The replacements are


"and ones(T, dims)" -> "and ones(T, dims)?

Sacha0 · 2017-12-18T20:31:49Z

NEWS.md

    consider `one(A)` ([#24656]).

+  * `ones(dims...)` `and ones(T, dims)` have been deprecated. The replacements are
+    `fill(1.0, dims...)` and `fill(one(T), dims...)`, but other common patterns, such as


Perhaps mention using literals also for other common types, e.g. fill(1, dims...) for Ints, fill(1f0, dims...) for Float32, e.g. fill(1+0im, dims...) for complex types, fill(0x1, dims...) for UInt8s, e.g. fill(1m, dims...) for unitful types, et cetera? And perhaps fill(T(1), dims...) as another alternative alongside fill(one(T), dims...)?

Sacha0 · 2017-12-18T20:34:03Z

base/array.jl


 Create an `Array`, with element type `T`, of all zeros with size specified by `dims`.
-See also [`ones`](@ref), [`similar`](@ref).
+See also [`similar`](@ref).


Perhaps add a reference to fill?

Sacha0 · 2017-12-18T20:38:02Z

base/linalg/qr.jl

-    xmin = ones(T, 1)
-    xmax = ones(T, 1)
+    xmin = fill(one(T), 1)
+    xmax = fill(one(T), 1)


Perhaps vector literals?

Sacha0 · 2017-12-18T20:40:15Z

doc/src/manual/noteworthy-differences.md

    be reversed in Julia relative to NumPy (see relevant section of [Performance Tips](@ref man-performance-tips)).
  * Julia's updating operators (e.g. `+=`, `-=`, ...) are *not in-place* whereas NumPy's are. This
-    means `A = ones(4); B = A; B += 3` doesn't change values in `A`, it rather rebinds the name `B`
+    means `A = fill(1, 4); B = A; B .+= 3` doesn't change values in `A`, it rather rebinds the name `B`


Good catch re. +=!

Ah, I hadn't read the rest of the sentence :). += is indeed correct, rather than .+=?

Right, the reason I changed was that vector + scalar is deprecated, so perhaps we should just rewrite the example completely.
Edit: Done.

Perfect! :)

Sacha0 · 2017-12-18T20:41:03Z

stdlib/Distributed/test/distributed_exec.jl

-global mrefs = TestSerCnt(ones(10))
-@test remotecall_fetch(()->(mrefs.v, 2*mrefs.v, 3*mrefs.v), id_other) == (ones(10), 2*ones(10), 3*ones(10))
+global mrefs = TestSerCnt(fill(1.,10))
+@test remotecall_fetch(()->(mrefs.v, 2*mrefs.v, 3*mrefs.v), id_other) == (fill(1.,10), fill(2.,10), fill(3.,10))


Sacha0 · 2017-12-18T20:48:07Z

test/arrayops.jl

    @test X[2,3] == 8
    @test X[4,5] == 14
-    @test isequal(ones(2,3) * ones(2,3)', [3. 3.; 3. 3.])
+    @test isequal(fill(3.,2,2), [3. 3.; 3. 3.])


I wonder why this test exists under the heading "comprehensions" and what purpose it serves :).

Sacha0 · 2017-12-21T19:25:47Z

base/abstractarray.jl

 # Examples
 ```jldoctest
-julia> A = ones(2,3,4);
+julia> A = fill(1,2,3,4);


fill(1,2,3,4) -> fill(1, (2,3,4)) for these examples? :)

StefanKarpinski · 2017-12-21T19:39:58Z

Sure, give it a shot. We'll see how much kicking and screaming there is.

JeffBezanson · 2017-12-28T20:35:50Z

Triage likes replacing the uses of ones in Base code and documentation, but is less sure about the deprecation. Let's at least get that commit merged.

fredrikekre · 2017-12-28T23:10:38Z

Rebased and dropped the last commit for later.

Sacha0 · 2018-01-03T03:59:24Z

base/array.jl


 Create an `Array`, with element type `T`, of all zeros with size specified by `dims`.
-See also [`ones`](@ref), [`similar`](@ref).
+See also [`fill`](@ref), [`ones`](@ref).


Why ref fill and ones (rather than fill and similar)?

The reference to similar here should have been removed as part of #24656/#24863. I assume the reason we referenced it here was because previously we had both zeros(A) and similar(A) doing similar things for A::AbstractArray.

Sacha0 · 2018-01-03T03:59:54Z

base/array.jl


 Create an `Array`, with element type `T`, of all ones with size specified by `dims`.
-See also [`zeros`](@ref), [`similar`](@ref).
+See also: [`fill`](@ref), [`zeros`](@ref).


Similarly here, why ref fill and zeros (rather than fill and similar)?

Sacha0 · 2018-01-03T04:10:17Z

stdlib/SuiteSparse/test/cholmod.jl

+    x = fill(1., 5)
+    @test cholfact(sparse(Float16(1)I, 5, 5))\x == x
+    @test cholfact(Symmetric(sparse(Float16(1)I, 5, 5)))\x == x
+    @test cholfact(Hermitian(sparse(Complex{Float16}(1)I, 5, 5)))\x == fill(1.0+0im, 5)


Could simplify the RHS to x I think?

Sacha0

Looks great! Much thanks for seeing this work through @fredrikekre! :)

fredrikekre · 2018-01-04T19:31:48Z

Linking #25392 (comment) but with zeros replace with ones.

…25087)

fredrikekre added arrays [a, r, r, a, y, s] deprecation This change introduces or involves a deprecation labels Dec 14, 2017

fredrikekre requested a review from Sacha0 December 14, 2017 23:22

fredrikekre force-pushed the reones branch from b290391 to 59135f2 Compare December 14, 2017 23:24

fredrikekre force-pushed the reones branch 4 times, most recently from 8037c99 to 7da6987 Compare December 18, 2017 12:15

Sacha0 reviewed Dec 18, 2017

View reviewed changes

fredrikekre force-pushed the reones branch 3 times, most recently from 8cf78a2 to 6db0c9b Compare December 21, 2017 17:12

Sacha0 added the triage This should be discussed on a triage call label Dec 21, 2017

Sacha0 reviewed Dec 21, 2017

View reviewed changes

fredrikekre force-pushed the reones branch from 6db0c9b to f228eda Compare December 21, 2017 22:20

fredrikekre force-pushed the reones branch from f228eda to e791413 Compare December 28, 2017 23:10

fredrikekre force-pushed the reones branch 3 times, most recently from eec1872 to 273c6c3 Compare January 1, 2018 00:15

fredrikekre changed the title ~~use fill (aka deprecate ones)~~ rewrite all calls to ones(...) Jan 2, 2018

Sacha0 reviewed Jan 3, 2018

View reviewed changes

fredrikekre force-pushed the reones branch 2 times, most recently from f67d64b to 2065aa7 Compare January 3, 2018 06:59

rewrite all ones(...) calls in base, stdlib, test and documentation

d222d20

fredrikekre force-pushed the reones branch from 2065aa7 to d222d20 Compare January 3, 2018 09:12

Sacha0 approved these changes Jan 4, 2018

View reviewed changes

fredrikekre mentioned this pull request Jan 4, 2018

rewrite all zeros(...) calls #25392

Closed

JeffBezanson merged commit b5d4f3c into JuliaLang:master Jan 4, 2018

JeffBezanson removed the triage This should be discussed on a triage call label Jan 4, 2018

fredrikekre deleted the reones branch January 4, 2018 22:03

fredrikekre mentioned this pull request Jan 11, 2018

at-deprecate ones([T,] dims...) #25507

Closed

JeffBezanson mentioned this pull request Sep 13, 2018

zeros etc. is arguably broken for mutable element types #29168

Closed

Keno pushed a commit that referenced this pull request Jun 5, 2024

rewrite all ones(...) calls in base, stdlib, test and documentation (#…

a51faa9

…25087)

Uh oh!

rewrite all calls to ones(...) #25087

rewrite all calls to ones(...) #25087

Uh oh!

Conversation

fredrikekre commented Dec 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sacha0 commented Dec 14, 2017

Uh oh!

Sacha0 commented Dec 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sacha0 Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrikekre Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanKarpinski commented Dec 21, 2017

Uh oh!

JeffBezanson commented Dec 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredrikekre commented Dec 28, 2017

Uh oh!

Sacha0 Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sacha0 left a comment

Choose a reason for hiding this comment

Uh oh!

fredrikekre commented Jan 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fredrikekre commented Dec 14, 2017 •

edited

Loading

Sacha0 commented Dec 14, 2017 •

edited

Loading

Sacha0 Dec 18, 2017 •

edited

Loading

fredrikekre Jan 3, 2018 •

edited

Loading

JeffBezanson commented Dec 28, 2017 •

edited

Loading

Sacha0 Jan 3, 2018 •

edited

Loading