Convert all remaining Generators to prefer statically-dimensioned Inputs and Output where possible #6620
Conversation
…uts and Output where possible
| Input<Buffer<>> input_a{"input_a", 4}; | ||
| Input<Buffer<>> input_b{"input_b", 4}; | ||
| Output<Buffer<>> output{"output", 4}; | ||
| Input<Buffer<void, 4>> input_a{"input_a"}; |
There was a problem hiding this comment.
This is a little unfortunate. Is it possible to make Buffer<4> work? I don't think that's doable in c++, right?
There was a problem hiding this comment.
No, definitely not. IMHO this is a minor price to pay.
| // A model weight is either just an input, or an input and an output | ||
| // (the updated weights and the ADAM state) depending on whether we're | ||
| // doing inference or training. | ||
| template<bool training> |
There was a problem hiding this comment.
In this file, runtime dimensionality seems much cleaner (e.g. there's value in having all the types be the same down below)
There was a problem hiding this comment.
We could selectively revert this one. (I did the conversion by deleting all the old ctors temporarily and then fixing everything.)
There was a problem hiding this comment.
+1 to selectively reverting this one
There was a problem hiding this comment.
Before I revert this, can I ask what you mean by "having all the types be the same down below"? Looking at this again, I don't know which types you mean, and so I'm confused about the specific nature of your concern. (I'm trying to understand why you feel the old approach is cleaner, so I can factor different views into future work.)
|
Tuesday Morning Review Ping |
| &head2_filter, &head2_bias, | ||
| &filter1, &bias1}; | ||
|
|
||
| for (Weight *w : weights) { |
There was a problem hiding this comment.
It's losing the ability to iterate over the weights here that I was referring to.
There was a problem hiding this comment.
That could be fixed pretty trivially by making backprop() a virtual method and adding a ModelWeightBase class.
There was a problem hiding this comment.
(Just pushed a revision to do this.)
…puts and Output where possible This is the same as #6620, except that it omits autoschedulers/adams2019/cost_model_generator.cpp (which is unusually complex and not yet settled as to whether the changes are welcome). Basically an attempt to land the uncontroversial parts.
No description provided.