DickJC123 edited a comment on issue #15657: Eliminate common expressions URL: https://github.com/apache/incubator-mxnet/pull/15657#issuecomment-531360012 Another area to spend some extra thought on involves the primary outputs and the gradients of primary inputs. If you combined two output NDArrays via CSE, would there be any problem with someone wanting to index through the outputs? Ditto for input grads. If a CSE-optimized model generated NDArrays for another model (and that model had mutable inputs), would functionality be preserved? I think a downstream model gets a copy of the NDArray as the input, so it should work, but do we ever want to optimize away that copy? CSE may eliminate downstream in-place options for node outputs that have been combined. Not sure if there's a case where using CSE is a net perf loss, but I doubt it. And finally, here's a fairly obscure case where the operator state may not be contained entirely in the node attributes (we should probably outlaw this usage, since it assumes no caching of the env var lookup): ``` sym = ... os.environ['MXNET_MY_OP_BEHAVIOR']=X sym2 = MyOp(data=sym,...) # op behavior affected by captured env var setting os.environ['MNXET_MY_OP_BEHAVIOR']=Y sym3 = MyOp(data=sym,...) # op behavior affected by captured env var setting ``` Perhaps shifting from the node attr dict to the param struct would catch this case.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
