Make outputsize understand multiple inputs

[mcabbott]

Closes #1466

[darsnack]

No complaints from me. I like the more informative error message. We'll have to make sure it stays in sync with the actual implementation of Chain, but I think it is worth it.

[mcabbott]

Great. As long as it still seems fast enough for your applications, that's one possible concern. It is only the outermost Chain, with no attempt to recurse into try/catch-ing sub-sub-layers.

[darsnack]

Good point, I'll double check the speed like we did on the original PR. But I think for its use case, a minor slowdown is fine for convenient errors. It doesn't need to be super fast, just faster than evaluating ResNet-101 (which this should be).

[darsnack]

Sorry for the delay. Here are the benchmarks on a large model:

julia> m = densenet_121();

# this PR
julia> @benchmark Flux.outputsize($m, (256, 256, 3, 50))
BenchmarkTools.Trial:
  memory estimate:  201.89 KiB
  allocs estimate:  4546
  --------------
  minimum time:     402.313 ms (0.00% GC)
  median time:      410.185 ms (0.00% GC)
  mean time:        423.873 ms (0.00% GC)
  maximum time:     480.239 ms (0.00% GC)
  --------------
  samples:          12
  evals/sample:     1

# Flux#master
julia> @benchmark Flux.outputsize($m, (256, 256, 3, 50))
BenchmarkTools.Trial:
  memory estimate:  278.48 KiB
  allocs estimate:  6634
  --------------
  minimum time:     416.605 ms (0.00% GC)
  median time:      417.849 ms (0.00% GC)
  mean time:        419.333 ms (0.00% GC)
  maximum time:     431.575 ms (0.00% GC)
  --------------
  samples:          12
  evals/sample:     1

I think based on that, it's good to merge.

[darsnack]

Think we still need someone with merge rights like @CarloLucibello

[mcabbott]

Excellent, thanks for checking.

I'm not super-sure how rights work, or whether merging is blocked to anyone not called Bors?

[mcabbott]

bors r+

[darsnack]

Yeah I had the same confusion. All I know is Github says I don't have merge rights, but I don't know if I'm allowed to call bors or not.

[CarloLucibello]

I now see "1 approving review by reviewers with write access", so Kyle's rights have been fixed and hopefully he should also be able to call bors. want to try calling bors yourself @mcabbott ?

[CarloLucibello]

there you go

[darsnack]

Thanks Carlo!

[CarloLucibello]

yeah merging is blocked also for me here, can go only through bors

[darsnack]

And my reading of ColPrac says that since Michael has write access only he is allowed to call bors after approval from someone else? (since he is the PR author)

[CarloLucibello]

And my reading of ColPrac says that since Michael has write access only he is allowed to call bors after approval from someone else? (since he is the PR author)

yes, and this is enforced by current github's settings. Those settings are actually stricter than ColPrac since for colprac also an approval from someone without write access is enough

[mcabbott]

OK, great. The direct merge button is still disabled for me, so I guess the enforcement is that Bors would not have listened to me, before the approval?

[CarloLucibello]

OK, great. The direct merge button is still disabled for me, so I guess the enforcement is that Bors would not have listened to me, before the approval?

that or it does its run then fails to merge, can't remember which one

[bors]

Build succeeded:

• buildkite/flux-dot-jl

[mcabbott]

If I'm reading the rules correctly, I may also merge PRs by other people, without another member's approval?

Such as #1009, which just celebrated its first birthday...

[CarloLucibello]

yes. approve then call bors

[mcabbott]

Ok, I tried. It says I need to be made a reviewer?