Skip to content

Commit

Permalink
Add verbose mode to isEqual check for easier debugging. (#3054)
Browse files Browse the repository at this point in the history
Summary:
Add a verbose mode to isEqual so that when there is a mismatch during our unit tests we log info the error. I disabled this in the two places we aren't using this in unit tests and so we want to early exit when Tensors aren't equal, i.e. GraphOptimizer and PlaceholderBindings.
Pull Request resolved: #3054

Differential Revision: D15702217

Pulled By: jfix71

fbshipit-source-id: 2d2d419b84b7445e60955bbce2a9e96a8edbdaa0
  • Loading branch information
jfix71 authored and facebook-github-bot committed Jun 7, 2019
1 parent fd54df2 commit 2146fdf
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 18 deletions.
54 changes: 38 additions & 16 deletions include/glow/Base/Tensor.h
Expand Up @@ -381,8 +381,11 @@ class Tensor final {
std::string toString(unsigned maxNumElem) const;

/// \returns true if the content of the other tensor \p other is identical to
/// this one.
bool isEqual(const Tensor &other, float allowedError = 0.0001) const {
/// this one, given some \p allowedError. If \p verbose and the tensors are
/// not equal, then we will log information about the mismatch (number of
/// elements exceeding allowed error; maximum error and location found; etc.).
bool isEqual(const Tensor &other, float allowedError = 0.0001,
bool verbose = true) const {
if (other.dims() != dims()) {
return false;
}
Expand All @@ -400,44 +403,44 @@ class Tensor final {

switch (getElementType()) {
case ElemKind::FloatTy:
return isEqualImpl<float>(other, allowedError);
return isEqualImpl<float>(other, allowedError, verbose);
case ElemKind::Float16Ty:
return isEqualImpl<float16_t>(other, allowedError);
return isEqualImpl<float16_t>(other, allowedError, verbose);
case ElemKind::Int8QTy:
assert(getType().getScale() == other.getType().getScale() &&
"Scales must match.");
assert(getType().getOffset() == other.getType().getOffset() &&
"Offsets must match.");
return isEqualImpl<int8_t>(other, allowedError);
return isEqualImpl<int8_t>(other, allowedError, verbose);
case ElemKind::UInt8QTy:
assert(getType().getScale() == other.getType().getScale() &&
"Scales must match.");
assert(getType().getOffset() == other.getType().getOffset() &&
"Offsets must match.");
return isEqualImpl<uint8_t>(other, allowedError);
return isEqualImpl<uint8_t>(other, allowedError, verbose);
case ElemKind::Int16QTy:
assert(getType().getScale() == other.getType().getScale() &&
"Scales must match.");
assert(getType().getOffset() == other.getType().getOffset() &&
"Offsets must match.");
return isEqualImpl<int16_t>(other, allowedError);
return isEqualImpl<int16_t>(other, allowedError, verbose);
case ElemKind::Int32QTy:
assert(getType().getScale() == other.getType().getScale() &&
"Scales must match.");
assert(getType().getOffset() == other.getType().getOffset() &&
"Offsets must match.");
return isEqualImpl<int32_t>(other, allowedError);
return isEqualImpl<int32_t>(other, allowedError, verbose);
case ElemKind::Int32ITy:
return isEqualImpl<int32_t>(other, allowedError);
return isEqualImpl<int32_t>(other, allowedError, verbose);
case ElemKind::Int64ITy:
return isEqualImpl<int64_t>(other, allowedError);
return isEqualImpl<int64_t>(other, allowedError, verbose);
// Note: We can use isEqualImpl() here because the scales/offsets will be
// compared as if they were data, so we will return false if any rowwise
// scale/offset do not match.
case ElemKind::UInt8FusedQTy:
return isEqualImpl<uint8_t>(other, allowedError);
return isEqualImpl<uint8_t>(other, allowedError, verbose);
case ElemKind::BoolTy:
return isEqualImpl<bool>(other, allowedError);
return isEqualImpl<bool>(other, allowedError, verbose);
}

// This is to make compiler happy. It can never reach this point as switch
Expand Down Expand Up @@ -559,18 +562,37 @@ class Tensor final {
}

template <class ElemTy>
bool isEqualImpl(const Tensor &other, float allowedError) const {
bool isEqualImpl(const Tensor &other, float allowedError,
bool verbose) const {
auto const *myData = getRawDataPointer<ElemTy>();
auto const *otherData = other.getRawDataPointer<ElemTy>();
double maxFoundError = 0.0;
size_t maxFoundErrorIdx = 0, numExceedingError = 0;
for (size_t i = 0, e = size(); i < e; i++) {
double delta = myData[i] - otherData[i];
delta = std::abs(delta);
// Since any comparison with NAN returns false, we use a negated condition
// so that this function correctly returns false when delta is NAN.
if (!(std::abs(delta) <= allowedError)) {
return false;
if (!(delta <= allowedError)) {
if (!verbose) {
return false;
}
numExceedingError += 1;
if (!(delta <= maxFoundError)) {
maxFoundError = delta;
maxFoundErrorIdx = i;
}
}
}
return true;
if (numExceedingError != 0) {
LOG(INFO) << "Tensors not equal: " << numExceedingError << " out of "
<< size() << " elements exceeded allowed error threshold "
<< allowedError << ". Maximum error found was " << maxFoundError
<< " at index " << maxFoundErrorIdx << ": "
<< myData[maxFoundErrorIdx] << " vs. "
<< otherData[maxFoundErrorIdx];
}
return numExceedingError == 0;
}
};

Expand Down
7 changes: 7 additions & 0 deletions include/glow/Support/Float16.h
Expand Up @@ -19,6 +19,7 @@
#include "fp16.h"

#include <cstdint>
#include <iostream>

namespace glow {

Expand Down Expand Up @@ -67,6 +68,12 @@ class float16 {
operator long long() const { return static_cast<long long>(data_); }
}; // End class float16.

/// Allow float16_t to be passed to an ostream.
inline std::ostream &operator<<(std::ostream &os, const float16 &b) {
os << float(b);
return os;
}

} // End namespace glow.

#endif // GLOW_SUPPORT_FLOAT16_H
4 changes: 3 additions & 1 deletion lib/Graph/PlaceholderBindings.cpp
Expand Up @@ -50,7 +50,9 @@ bool PlaceholderBindings::compare(const PlaceholderBindings *A,
const auto *tensorB =
B->get(B->getPlaceholderByName(placeholder->getName()));

if (!tensorA || !tensorB || !tensorA->isEqual(*tensorB)) {
if (!tensorA || !tensorB ||
!tensorA->isEqual(*tensorB, /* allowedError */ 0.0001,
/* verbose */ false)) {
return false;
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Optimizer/GraphOptimizer.cpp
Expand Up @@ -1686,7 +1686,8 @@ struct ConstsEqDedup {
// Only dedup Constants if their data matches exactly, so allowed error is
// 0.0.
return lhs->getPayload().isEqual(rhs->getPayload(),
/* allowedError */ 0.0);
/* allowedError */ 0.0,
/* verbose */ false);
}
};

Expand Down

0 comments on commit 2146fdf

Please sign in to comment.