apeforest commented on a change in pull request #14836: Refactor AGInfo and
Imperative
URL: https://github.com/apache/incubator-mxnet/pull/14836#discussion_r305024247
##########
File path: src/imperative/imperative.cc
##########
@@ -273,40 +275,34 @@ void Imperative::RecordOp(
info.outputs.back().dtype_ = outputs[i]->dtype();
info.outputs.back().storage_type_ = outputs[i]->storage_type();
}
- outputs[i]->entry_ = nnvm::NodeEntry{node, i, 0};
+ outputs[i]->entry_ = nnvm::NodeEntry{node, static_cast<uint32_t>(i), 0};
}
}
-std::vector<NDArray*> Imperative::Backward(
- const std::vector<NDArray*>& outputs,
- const std::vector<NDArray*>& ograds,
- const std::vector<NDArray*>& variables,
- bool is_train, bool retain_graph,
- bool create_graph) {
- using namespace nnvm;
- using namespace imperative;
- static const std::vector<const Op*> zero_ops{Op::Get("zeros_like"),
Op::Get("_zeros")};
- static const Op* copy_op = Op::Get("_copy");
-
- // Construct forward graph
- Graph graph;
- graph.outputs.reserve(outputs.size());
- for (const auto& i : outputs) {
+std::vector<nnvm::NodeEntry> Imperative::CreateForwardGraph(const
std::vector<NDArray*>& outputs) {
Review comment:
I found this function and naming quite confusing. It is not really creating
a graph, but simply assign a vector of outputs to the graph outputs. Maybe
consider creating and returning the graph itself from this function?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
