Remove the training logic and visibility from Variable.

include/glow/ExecutionEngine/ExecutionEngine.h

@@ -89,11 +89,8 @@ class ExecutionEngine final {
 //         Helper methods for running the execution engine.
 //===----------------------------------------------------------------------===//
 
-/// This method updates the variables in \p vars with the tensor content
-/// values \p inputs.
-void updateVariables(llvm::ArrayRef<Variable *> vars,
-                     llvm::ArrayRef<Tensor *> inputs);
-
+/// This method updates the placeholders in \p ph with the tensor content
+/// values \p inputs, in \p ctx.
 void updateVariables(Context &ctx, llvm::ArrayRef<Placeholder *> ph,
                      llvm::ArrayRef<Tensor *> inputs);
 
@@ -104,14 +101,6 @@ void updateInputsByName(Context &ctx, Module *mod,
                         llvm::ArrayRef<llvm::StringRef> ph,
                         llvm::ArrayRef<Tensor *> inputs);
 
-/// Update the content of the tensors \p vars with some slices that from \p
-/// inputs. The data starts at slice \p sampleIdx and wraps around until the
-/// data in \p v is filled. All dimensions, except for the first (batch)
-/// dimension must be identical.
-void updateVariablesFromBatch(llvm::ArrayRef<Variable *> vars,
-                              llvm::ArrayRef<Tensor *> inputs,
-                              size_t sampleIdx);
-
 /// Runs \p iterations iterations of the compiled function. The method updates a
 /// global counter and future invocations of this method continue running
 /// iterations of the batch at the next available slice.
@@ -124,9 +113,6 @@ void updateVariablesFromBatch(llvm::ArrayRef<Variable *> vars,
 /// variable records the number of samples that were consumed by the network in
 /// previous iterations. The next input to be loaded is
 /// (sampleCounter % batchsize).
-void runBatch(ExecutionEngine &EE, size_t iterations, size_t &sampleCounter,
-              llvm::ArrayRef<Variable *> vars, llvm::ArrayRef<Tensor *> inputs);
-
 void runBatch(ExecutionEngine &EE, Context &ctx, size_t iterations,
               size_t &sampleCounter, llvm::ArrayRef<Placeholder *> ph,
               llvm::ArrayRef<Tensor *> inputs);

include/glow/Graph/Graph.h

@@ -141,23 +141,15 @@ class Module final {
                                  float scale, int32_t offset,
                                  llvm::StringRef name, bool isTrainable);
 
-  Variable *createVariable(TypeRef T, llvm::StringRef name,
-                           VisibilityKind visibility = VisibilityKind::Private,
-                           bool isTrainable = true);
+  Variable *createVariable(TypeRef T, llvm::StringRef name);
 
   Variable *createVariable(ElemKind T, llvm::ArrayRef<size_t> dims,
-                           llvm::StringRef name,
-                           VisibilityKind visibility = VisibilityKind::Private,
-                           bool isTrainable = true);
+                           llvm::StringRef name);
 
   Variable *createVariable(ElemKind T, llvm::ArrayRef<size_t> dims, float scale,
-                           int32_t offset, llvm::StringRef name,
-                           VisibilityKind visibility = VisibilityKind::Private,
-                           bool isTrainable = true);
+                           int32_t offset, llvm::StringRef name);
 
-  Variable *createVariable(llvm::StringRef name, const Tensor &tensor,
-                           VisibilityKind visibility = VisibilityKind::Private,
-                           bool isTrainable = true);
+  Variable *createVariable(llvm::StringRef name, const Tensor &tensor);
 
   ///@}
 

include/glow/Graph/Nodes.h

@@ -31,12 +31,8 @@ namespace glow {
 // Storage is the base class for Variables, which are bound to tensors, and
 // Placeholder nodes which are unbound.
 class Storage : public Node {
-  /// Specifies if the variable or placeholder is trainable.
-  bool isTrainable_;
-
 public:
-  Storage(Kinded::Kind k, llvm::StringRef name, bool isTrainable)
-      : Node(k, name), isTrainable_(isTrainable) {}
+  Storage(Kinded::Kind k, llvm::StringRef name) : Node(k, name) {}
 
   /// \return the single output value of the node.
   NodeValue getOutput() { return getNthResult(0); }
@@ -54,10 +50,6 @@ class Storage : public Node {
   Node *clone() const;
   /// @}
 
-  /// \returns True if the Variable or placeholder are trainable during
-  /// differentiation.
-  bool isTraining() const { return isTrainable_; }
-
   /// \returns result type of the variable.
   TypeRef getType() const { return Node::getType(0); }
 
@@ -74,37 +66,27 @@ class Storage : public Node {
 };
 
 class Variable : public Storage {
-  /// Specifies the visibility of the variable.
-  VisibilityKind visibility_;
   /// The tensor payload that the variable holds.
   Tensor payload_;
 
 public:
   /// Create a new variable and initialize its payload.
-  Variable(llvm::StringRef name, TypeRef Ty, VisibilityKind visibility,
-           bool isTrainable)
-      : Storage(Kinded::Kind::VariableKind, name, isTrainable),
-        visibility_(visibility) {
+  Variable(llvm::StringRef name, TypeRef Ty)
+      : Storage(Kinded::Kind::VariableKind, name) {
     addResult(Ty);
     payload_.reset(*Ty);
   }
 
-  Variable(llvm::StringRef name, VisibilityKind visibility, Tensor &&payload)
-      : Storage(Kinded::Kind::VariableKind, name, false),
-        visibility_(visibility), payload_(std::move(payload)) {
+  Variable(llvm::StringRef name, Tensor &&payload)
+      : Storage(Kinded::Kind::VariableKind, name),
+        payload_(std::move(payload)) {
     addResult(&payload_.getType());
   }
 
-  /// \returns True if the Variable is private.
-  bool isPrivate() const { return visibility_ == VisibilityKind::Private; }
-
   static bool classof(const Kinded *k) {
     return k->getKind() == Kinded::Kind::VariableKind;
   }
 
-  /// \returns the visibility of the variable.
-  VisibilityKind getVisibilityKind() const { return visibility_; }
-
   Tensor &getPayload() { return payload_; }
 
   const Tensor &getPayload() const { return payload_; }
@@ -124,13 +106,21 @@ class Variable : public Storage {
 /// this node at runtime. Placeholders are used as inputs and output nodes to
 /// the network.
 class Placeholder : public Storage {
+  /// Specifies if the variable or placeholder is trainable.
+  bool isTrainable_;
+
 public:
   /// Create a new placeholder variable.
   Placeholder(llvm::StringRef name, TypeRef Ty, bool isTrainable)
-      : Storage(Kinded::Kind::PlaceholderKind, name, isTrainable) {
+      : Storage(Kinded::Kind::PlaceholderKind, name),
+        isTrainable_(isTrainable) {
     addResult(Ty);
   }
 
+  /// \returns True if the Variable or placeholder are trainable during
+  /// differentiation.
+  bool isTraining() const { return isTrainable_; }
+
   static bool classof(const Kinded *k) {
     return k->getKind() == Kinded::Kind::PlaceholderKind;
   }

include/glow/Importer/CommonOperatorLoader.h

@@ -119,8 +119,7 @@ class CommonOperatorLoader : public ProtobufLoader {
     // have an option for a selected input anyway. So I am creating this as a
     // placeholder which goes unused during inference.
     auto selected = G_.getParent()->createVariable(
-        ElemKind::Int64ITy, {in.dims()[0], 1}, "selected",
-        VisibilityKind::Private, false);
+        ElemKind::Int64ITy, {in.dims()[0], 1}, "selected");
 
     // ONNX allows shapes like <N x 10 x 1 x 1 >. Flatten the inputs to the
     // softmax function. This is similar to a bitcast operation.

lib/Backends/CPU/AllocationsInfo.cpp

@@ -45,8 +45,6 @@ void AllocationsInfo::allocateWeightVars(const IRFunction *F,
   for (auto &v : F->getGraph()->getParent()->getVars()) {
     assert(isa<WeightVar>(F->getWeightForNode(v)));
     auto *w = cast<WeightVar>(F->getWeightForNode(v));
-    if (v->getVisibilityKind() == VisibilityKind::Public)
-      continue;
     auto numBytes = w->getSizeInBytes();
     size_t addr = constantWeightVarsAllocator.allocate(numBytes, w);
     if (!absoluteAddr) {
@@ -58,23 +56,6 @@ void AllocationsInfo::allocateWeightVars(const IRFunction *F,
     }
   }
 
-  // Process all mutable WeightVars afterwards.
-  for (auto &v : F->getGraph()->getParent()->getVars()) {
-    assert(isa<WeightVar>(F->getWeightForNode(v)));
-    auto *w = cast<WeightVar>(F->getWeightForNode(v));
-    if (v->getVisibilityKind() != VisibilityKind::Public)
-      continue;
-    auto numBytes = w->getSizeInBytes();
-    size_t addr = mutableWeightVarsAllocator.allocate(numBytes, w);
-    if (!absoluteAddr) {
-      allocatedAddressed_[w] = addr;
-    } else {
-      // Reuse the address used by the payload.
-      allocatedAddressed_[w] =
-          v->getPayload().getUnsafePtr() - static_cast<char *>(nullptr);
-    }
-  }
-
   // Allocate addresses for the Placeholders.
   for (auto PH : ctx.pairs()) {
     assert(isa<WeightVar>(F->getWeightForNode(PH.first)));
@@ -208,10 +189,7 @@ void AllocationsInfo::numberValues(const IRFunction *F) {
   for (auto &v : F->getGraph()->getParent()->getVars()) {
     assert(isa<WeightVar>(F->getWeightForNode(v)));
     auto *w = cast<WeightVar>(F->getWeightForNode(v));
-    auto kind = v->getVisibilityKind() != VisibilityKind::Public
-                    ? ValueKind::ConstantWeight
-                    : ValueKind::MutableWeight;
-    valueNumbers_[w] = std::make_pair(kind, valueIdx++);
+    valueNumbers_[w] = std::make_pair(ValueKind::ConstantWeight, valueIdx++);
   }
 
   // Assign numbers to all placeholders.

lib/Backends/CPU/BundleSaver.cpp

@@ -54,8 +54,6 @@ void BundleSaver::saveWeights(llvm::StringRef weightsFileName) {
   size_t maxPos = 0;
   for (auto &v : F_->getGraph()->getParent()->getVars()) {
     auto *w = cast<WeightVar>(F_->getWeightForNode(v));
-    if (v->getVisibilityKind() == VisibilityKind::Public)
-      continue;
     auto numBytes = w->getSizeInBytes();
     auto payload = v->getPayload().getUnsafePtr();
     auto addr = allocationsInfo_.allocatedAddressed_[w];
@@ -99,7 +97,6 @@ void BundleSaver::emitSymbolTable() {
   // size and kind.
   for (auto &v : F_->getGraph()->getParent()->getVars()) {
     auto *w = cast<WeightVar>(F_->getWeightForNode(v));
-    bool isConstWeight = v->getVisibilityKind() != VisibilityKind::Public;
     auto size = w->getType()->size();
     auto addr = allocationsInfo_.allocatedAddressed_[w];
     // Create an SymbolTableEntry.
@@ -114,7 +111,7 @@ void BundleSaver::emitSymbolTable() {
          // size.
          llvm::ConstantInt::get(sizeTTy, size),
          // kind.
-         llvm::ConstantInt::get(charTy, isConstWeight ? 0 : 1)});
+         llvm::ConstantInt::get(charTy, /*isConstWeight*/ 0)});
     entries.push_back(entry);
   }
 

lib/Backends/CPU/Transforms.cpp

@@ -44,7 +44,7 @@ static Node *optimizeCPUConv(ConvolutionNode *CN, Function *F) {
   }
 
   Variable *filter = dyn_cast<Variable>(CN->getFilter());
-  if (!filter || filter->getNumUsers() != 1 || !filter->isPrivate()) {
+  if (!filter || filter->getNumUsers() != 1) {
     // Can't mutate the filter.
     return nullptr;
   }
@@ -58,9 +58,9 @@ static Node *optimizeCPUConv(ConvolutionNode *CN, Function *F) {
   TypeRef filterTy = filter->getType();
   auto dims = filterTy->dims();
   assert(dims.size() == 4 && "Invalid filter size");
-  auto *filter8 = M->createVariable(
-      filterTy->getElementType(), {dims[0] / 8, dims[1], dims[2], dims[3], 8},
-      filter->getName(), VisibilityKind::Private, false);
+  auto *filter8 = M->createVariable(filterTy->getElementType(),
+                                    {dims[0] / 8, dims[1], dims[2], dims[3], 8},
+                                    filter->getName());
 
   auto F8H = filter8->getHandle();
   auto FH = filter->getHandle();

lib/ExecutionEngine/ExecutionEngine.cpp

@@ -41,26 +41,6 @@ void ExecutionEngine::setBackend(Backend *backend) {
 
 ExecutionEngine::~ExecutionEngine() = default;
 
-void glow::updateVariables(llvm::ArrayRef<Variable *> vars,
-                           llvm::ArrayRef<Tensor *> inputs) {
-  assert(inputs.size() == vars.size() &&
-         "The number of inputs does not match the number of variables");
-
-  // Update the input variables.
-  for (int i = 0, e = vars.size(); i < e; i++) {
-    assert(vars[i] && "Invalid value");
-    assert(vars[i]->getVisibilityKind() == VisibilityKind::Public &&
-           "Trying to update a private variable");
-    auto &t = vars[i]->getPayload();
-    auto dim = inputs[i]->dims();
-    (void)dim;
-    assert(t.dims() == dim &&
-           t.getElementType() == inputs[i]->getElementType() &&
-           "Mismatch on Variable and Tensor types.");
-    t.assign(inputs[i]);
-  }
-}
-
 void glow::updateVariables(Context &ctx, llvm::ArrayRef<Placeholder *> ph,
                            llvm::ArrayRef<Tensor *> inputs) {
   assert(inputs.size() == ph.size() &&
@@ -99,47 +79,6 @@ void ExecutionEngine::run() {
   function_->execute();
 }
 
-/// Update the content of the tensors \p vars with some slices that are from \p
-/// inputs. The data starts at slice \p sampleIdx and wraps around until the
-/// data in \p v is filled. All dimensions, except for the first (batch)
-/// dimension must be identical.
-void glow::updateVariablesFromBatch(llvm::ArrayRef<Variable *> vars,
-                                    llvm::ArrayRef<Tensor *> inputs,
-                                    size_t sampleIdx) {
-  assert(!inputs.empty() && "No inputs");
-  assert(inputs.size() == vars.size() &&
-         "The number of inputs does not match the number of variables");
-
-  // Update the input variables.
-  for (int i = 0, e = vars.size(); i < e; i++) {
-    assert(vars[i] && "Invalid value");
-    auto &t = vars[i]->getPayload();
-
-    auto dim = inputs[i]->dims();
-    assert(t.dims().drop_front() == dim.drop_front() && "Invalid slice size");
-    // Extract the n'th slice, that must be a tensor.
-    size_t slc = sampleIdx % dim[0];
-    t.copyConsecutiveSlices(inputs[i], slc);
-  }
-}
-
-void glow::runBatch(ExecutionEngine &EE, size_t iterations,
-                    size_t &sampleCounter, llvm::ArrayRef<Variable *> vars,
-                    llvm::ArrayRef<Tensor *> inputs) {
-  // This is the size of one batch (the number of samples in the batch).
-  size_t batchSize = vars[0]->getType()->dims()[0];
-
-  for (size_t i = 0; i < iterations; i++) {
-    // Pick up one slice from the input tensors, and load it into corresponding
-    // network Variables. Then, run a single pass over the network.
-    glow::updateVariablesFromBatch(vars, inputs, sampleCounter);
-
-    // Run the network.
-    EE.run();
-    sampleCounter += batchSize;
-  }
-}
-
 void glow::runBatch(ExecutionEngine &EE, Context &ctx, size_t iterations,
                     size_t &sampleCounter, llvm::ArrayRef<Placeholder *> ph,
                     llvm::ArrayRef<Tensor *> inputs) {