Skip to content

Fix forwardDims() processing order

Olivier BICHLER requested to merge Fix-forwardDims-processing-order into dev

Fixes issue aidge#115 (closed) "Inconsistent batch size after inference"), which uncovers a big problem with current implementation of forwardDims().

I propose to go back to an improved version of the previous, initial implementation, which walks through the graph from parents to childs.

The function has to verify the following conditions:

  • Every node must forwardDims() regardless of if dims were previously forwarded or not (this was not verified for the first implementation);
  • forwadDims() calls must be made in node dependencies order, because if dims have changed at any point in the graph, it must de propagated correctly to all succeeding nodes (this was not verified for the second implementation);
  • It must handle cyclic dependencies correctly (this was not verified for the first implementation).

I don't like very much the explicit dependency to Memorize_Op::Type in the function. But a specific case for this operator also already exists in the Scheduler. If at some point another operator capable of handling recursive connections would be needed, a more generic mechanism may be put in place...

Also fixes aidge#114 (closed) (remove redondant and useless universal reference overloads).

Edited by Olivier BICHLER

Merge request reports