{"action":"created","issue":{"url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/128","repository_url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl","labels_url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/128/labels{/name}","comments_url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/128/comments","events_url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/128/events","html_url":"https://github.com/JuliaStats/NullableArrays.jl/pull/128","id":164112611,"number":128,"title":"WIP: map for 0.5","user":{"login":"davidagold","id":11862683,"avatar_url":"https://avatars.githubusercontent.com/u/11862683?v=3","gravatar_id":"","url":"https://api.github.com/users/davidagold","html_url":"https://github.com/davidagold","followers_url":"https://api.github.com/users/davidagold/followers","following_url":"https://api.github.com/users/davidagold/following{/other_user}","gists_url":"https://api.github.com/users/davidagold/gists{/gist_id}","starred_url":"https://api.github.com/users/davidagold/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidagold/subscriptions","organizations_url":"https://api.github.com/users/davidagold/orgs","repos_url":"https://api.github.com/users/davidagold/repos","events_url":"https://api.github.com/users/davidagold/events{/privacy}","received_events_url":"https://api.github.com/users/davidagold/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2016-07-06T16:02:19Z","updated_at":"2016-07-07T16:33:14Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/pulls/128","html_url":"https://github.com/JuliaStats/NullableArrays.jl/pull/128","diff_url":"https://github.com/JuliaStats/NullableArrays.jl/pull/128.diff","patch_url":"https://github.com/JuliaStats/NullableArrays.jl/pull/128.patch"},"body":"This is an attempt to transition away from the metaprogramming-heavy approach to map we took for Julia v0.4. Admittedly, this does introduce the use of generated functions in n-arg `map`/`map!`. Does anybody have any suggestions as to how best maintain compatibility with 0.4? What I have here seems kind of crude.\r\n\r\nThere's a lot of code repetition due to the 1-arg, 2-arg and n-arg methods for `map`. The Base `map(f, A)` regime calls `collect_similar(A, Generator(f, A))`, but I haven't found a way to `collect` over a `Generator{NullableArray, F}` without creating a lot of unnecessary `Nullable` objects -- so, hooking into that interface isn't really an option. I'm not sure the best way to reduce the code footprint here.\r\n\r\nBrief performance summary:\r\n```julia\r\nn = 1_000_000\r\nA = rand(n)\r\nM = rand(Bool, n)\r\nX = NullableArray(A)\r\nY = NullableArray(A, M)\r\nAs = [ rand(n) for i in 1:5 ]\r\nMs = [ rand(Bool, n) for i in 1:5 ]\r\nXs = [ NullableArray(A) for A in As ]\r\nYs = [ NullableArray(A, M) for (A, M) in zip(As, Ms)]\r\nf(x) = 5*x\r\nf(xs...) = prod(xs)\r\n\r\njulia> @benchmark map(f, A)\r\nBenchmarkTools.Trial: \r\n  samples:          1203\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  7.63 mb\r\n  allocs estimate:  4\r\n  minimum time:     2.77 ms (0.00% GC)\r\n  median time:      3.22 ms (0.00% GC)\r\n  mean time:        4.15 ms (22.39% GC)\r\n  maximum time:     73.10 ms (6.17% GC)\r\n\r\njulia> @benchmark map(f, X)\r\nBenchmarkTools.Trial: \r\n  samples:          171\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  8.58 mb\r\n  allocs estimate:  12\r\n  minimum time:     26.32 ms (0.00% GC)\r\n  median time:      29.16 ms (0.00% GC)\r\n  mean time:        29.31 ms (2.95% GC)\r\n  maximum time:     44.79 ms (5.40% GC)\r\n\r\njulia> @benchmark map(f, X; lift=true)\r\nBenchmarkTools.Trial: \r\n  samples:          968\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  8.58 mb\r\n  allocs estimate:  9\r\n  minimum time:     3.83 ms (0.00% GC)\r\n  median time:      4.30 ms (0.00% GC)\r\n  mean time:        5.16 ms (16.77% GC)\r\n  maximum time:     11.41 ms (0.00% GC)\r\n\r\njulia> @benchmark map(f, Y; lift=true)\r\nBenchmarkTools.Trial: \r\n  samples:          695\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  8.58 mb\r\n  allocs estimate:  9\r\n  minimum time:     5.98 ms (0.00% GC)\r\n  median time:      6.22 ms (0.00% GC)\r\n  mean time:        7.19 ms (11.75% GC)\r\n  maximum time:     18.70 ms (57.61% GC)\r\n\r\njulia> @benchmark map(f, As...)\r\nBenchmarkTools.Trial: \r\n  samples:          158\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  7.63 mb\r\n  allocs estimate:  10\r\n  minimum time:     27.83 ms (0.00% GC)\r\n  median time:      30.86 ms (0.00% GC)\r\n  mean time:        31.65 ms (2.66% GC)\r\n  maximum time:     40.85 ms (6.13% GC)\r\n\r\njulia> @benchmark map(f, Xs...)\r\nBenchmarkTools.Trial: \r\n  samples:          5\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  1.04 gb\r\n  allocs estimate:  20999008\r\n  minimum time:     1.06 s (12.26% GC)\r\n  median time:      1.07 s (12.69% GC)\r\n  mean time:        1.09 s (13.74% GC)\r\n  maximum time:     1.14 s (18.10% GC)\r\n\r\njulia> @benchmark map(f, Xs...; lift=true)\r\nBenchmarkTools.Trial: \r\n  samples:          354\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  8.58 mb\r\n  allocs estimate:  32\r\n  minimum time:     12.27 ms (0.00% GC)\r\n  median time:      13.62 ms (0.00% GC)\r\n  mean time:        14.18 ms (6.01% GC)\r\n  maximum time:     23.19 ms (10.96% GC)\r\n\r\njulia> @benchmark map(f, Ys...; lift=true)\r\nBenchmarkTools.Trial: \r\n  samples:          357\r\n  evals/sample:     1\r\n  time tolerance:   5.00%\r\n  memory tolerance: 1.00%\r\n  memory estimate:  8.58 mb\r\n  allocs estimate:  32\r\n  minimum time:     12.49 ms (0.00% GC)\r\n  median time:      13.35 ms (0.00% GC)\r\n  mean time:        14.02 ms (6.13% GC)\r\n  maximum time:     20.85 ms (19.69% GC)\r\n```\r\n\r\nIt seems pretty clear that, as far as performance goes, we ought to transition towards lifting by default. I think this seems reasonable for most statistics/data science-related use cases. To do so, would we need to deprecate the current behavior for a release cycle?\r\n\r\n@nalimilan @johnmyleswhite thoughts?"},"comment":{"url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/comments/231134194","html_url":"https://github.com/JuliaStats/NullableArrays.jl/pull/128#issuecomment-231134194","issue_url":"https://api.github.com/repos/JuliaStats/NullableArrays.jl/issues/128","id":231134194,"user":{"login":"nalimilan","id":1120448,"avatar_url":"https://avatars.githubusercontent.com/u/1120448?v=3","gravatar_id":"","url":"https://api.github.com/users/nalimilan","html_url":"https://github.com/nalimilan","followers_url":"https://api.github.com/users/nalimilan/followers","following_url":"https://api.github.com/users/nalimilan/following{/other_user}","gists_url":"https://api.github.com/users/nalimilan/gists{/gist_id}","starred_url":"https://api.github.com/users/nalimilan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nalimilan/subscriptions","organizations_url":"https://api.github.com/users/nalimilan/orgs","repos_url":"https://api.github.com/users/nalimilan/repos","events_url":"https://api.github.com/users/nalimilan/events{/privacy}","received_events_url":"https://api.github.com/users/nalimilan/received_events","type":"User","site_admin":false},"created_at":"2016-07-07T16:33:14Z","updated_at":"2016-07-07T16:33:14Z","body":"> w/r/t the allocations on non-lifted map: I thought these are due to all the Nullable objects being created by indexing. Or should they not show up? There may be a problem.\r\n\r\nNot sure. But since it's an immutable, it should be possible to avoid the allocation at least in some scenarios. For example, by inlining the function which creates the `Nullable` (the rest of the work could be wrapped in a `@noinline` function). You'd need to ask more expert people about that.\r\n\r\nAs regards being faster than Base's `map`: could you open an issue to discuss this, or ask for feedback on Slack?"}}
{"actor":{"display_login":"nalimilan"}}