Fix quantization when doing parallel clone testing; add check for bitwise accuracy (#3440)

Summary:
Previously we were only checking the results from parallel clones against the interpreter result within some threshold. With this change we will also check that all of the parallel clone results from the backend are bitwise accurate.

When implementing this I found that things were currently broken; the quantizer was not erroring out when it should have been (fixed in 1st commit). Additionally we were not copying over the quantization infos for the cloned nodes to find their correct quantization parameters, so without the check from the 1st commit those nodes were just not being quantized.
Pull Request resolved: #3440

Test Plan: All tests still pass with the added check; also ran OperatorTests with `--parallel-clone-count=10`.

Differential Revision: D16944262

Pulled By: jfix71

fbshipit-source-id: 117abf463c409b1202fdbefd1139509c8d85165c

Author: jfix71

lib/Quantization/Quantization.cpp

@@ -294,6 +294,11 @@ class FunctionQuantizer : public FunctionConverter {
       if (val.getElementType() == ElemKind::FloatTy) {
         needsQuantization = true;
         if (!quantizationParamsExist(val)) {
+          CHECK(!assertAllNodesQuantized_)
+              << "Quantization parameters did not exist for an input NodeValue "
+                 "that should have been quantized; input number "
+              << idx << " of node:\n"
+              << node.getDebugDesc();
           return false;
         }
       }
@@ -305,6 +310,11 @@ class FunctionQuantizer : public FunctionConverter {
       if (val.getElementType() == ElemKind::FloatTy) {
         needsQuantization = true;
         if (!quantizationParamsExist(val)) {
+          CHECK(!assertAllNodesQuantized_)
+              << "Quantization parameters did not exist for a result of a Node "
+                 "that should have been quantized; result number "
+              << idx << " of node:\n"
+              << node.getDebugDesc();
           return false;
         }
       }

tests/unittests/BackendTestUtils.cpp

@@ -159,12 +159,6 @@ void compareAgainstInterpreter(llvm::StringRef backendName,
   FunctionTensorPair IFT = createAndInitFunction(iBindings, IEE);
   FunctionTensorPair BFT = createAndInitFunction(bBindings, BEE);
 
-  // Clone the Function inside itself many times if desired.
-  std::unordered_set<Tensor *> resultTensors =
-      cloneFunInsideFun(BFT, &bBindings, count);
-  assert(resultTensors.size() == count &&
-         "Should get the same number of Tensors back as count.");
-
   Function *IF = IFT.first;
 
   // Set up the configs for interpreter and backend. If one or both functions
@@ -178,6 +172,12 @@ void compareAgainstInterpreter(llvm::StringRef backendName,
   CompilationContext &cctxI = configs.first;
   CompilationContext &cctxB = configs.second;
 
+  // Clone the Function inside itself many times if desired.
+  std::unordered_set<Tensor *> resultTensors =
+      cloneFunInsideFun(BFT, &bBindings, cctxB, count);
+  assert(resultTensors.size() == count &&
+         "Should get the same number of Tensors back as count.");
+
   BEE.compile(cctxB);
   BEE.run(bBindings);
 
@@ -188,10 +188,19 @@ void compareAgainstInterpreter(llvm::StringRef backendName,
   for (Tensor *T : resultTensors) {
     EXPECT_TRUE(IFT.second->isEqual(*T, allowedError));
   }
+
+  // Additionally check that each of the results from the parallel cloned
+  // Functions are bitwise equal.
+  auto it = resultTensors.begin();
+  Tensor *firstResult = *it;
+  for (it++; it != resultTensors.end(); it++) {
+    EXPECT_TRUE(firstResult->isBitwiseEqual(**it));
+  }
 }
 
 std::unordered_set<Tensor *> cloneFunInsideFun(FunctionTensorPair FTP,
                                                PlaceholderBindings *bindings,
+                                               CompilationContext &cctx,
                                                unsigned count) {
   Function *origF = FTP.first;
 
@@ -254,6 +263,26 @@ std::unordered_set<Tensor *> cloneFunInsideFun(FunctionTensorPair FTP,
   // Now erase the clone we used to copy in, as it's no longer needed.
   mod->eraseFunction(cloneF);
 
+  // Finally, duplicate all of the node quantization infos with the new expected
+  // clone's name so that the cloned copies will find the same quantization info
+  // as the original node if being quantized.
+  auto &origInfos = cctx.precisionConfig.quantConfig.infos;
+  origInfos.reserve(count * origInfos.size());
+  std::vector<NodeQuantizationInfo> newInfos;
+  newInfos.reserve((count - 1) * origInfos.size());
+  for (const auto &QI : origInfos) {
+    const size_t colonIdx = QI.nodeOutputName_.find(":");
+    assert(colonIdx != std::string::npos && "Name should always contain ':'");
+    for (size_t i = 1; i < count; i++) {
+      std::string newName(QI.nodeOutputName_);
+      // Cloned nodes end up with the original name plus the count number
+      // appended to their name due to uniquing. Replicate the same thing.
+      newName.insert(colonIdx, std::to_string(i));
+      newInfos.emplace_back(newName, QI.tensorQuantizationParams_);
+    }
+  }
+  origInfos.insert(origInfos.end(), newInfos.begin(), newInfos.end());
+
   return resultTensors;
 }
 

tests/unittests/BackendTestUtils.h

@@ -239,9 +239,12 @@ void compareAgainstInterpreter(
 /// Given some \p FTP representing a Function with a single SaveNode and its
 /// Tensor output, duplicate the Nodes in the Function and their Placeholder
 /// inputs given \p bindings \p parallelCount times. \returns a set of Tensor
-/// pointers for each output of the cloned Function.
+/// pointers for each output of the cloned Function. If the quantization node
+/// info found in \p cctx exists, then all of the node infos will be cloned
+/// accordingly with the names of the newly cloned nodes added to the Function.
 std::unordered_set<Tensor *> cloneFunInsideFun(FunctionTensorPair FTP,
                                                PlaceholderBindings *bindings,
+                                               CompilationContext &cctx,
                                                unsigned parallelCount);
 
 void inferConvNet(Tensor *inputs, Tensor *filter, Tensor *bias, Tensor *out,

tests/unittests/OperatorTest.cpp

@@ -960,13 +960,14 @@ TEST_P(OperatorTest, matmul_ParCloneTest10) {
   auto *save = F_->createSave("save", R);
   auto *saveTensor = bindings_.allocate(save->getPlaceholder());
 
+  CompilationContext cctx;
   const unsigned parallelCount = 10;
   auto resultTensors = cloneFunInsideFun(std::make_pair(F_, saveTensor),
-                                         &bindings_, parallelCount);
+                                         &bindings_, cctx, parallelCount);
 
   EXPECT_EQ(resultTensors.size(), parallelCount);
 
-  EE_.compile(CompilationMode::Infer);
+  EE_.compile(cctx);
   EE_.run(bindings_);
 
   for (Tensor *T : resultTensors) {