From cb0748a78e3b20a6b577b2da1b0eb7f00ed2bd81 Mon Sep 17 00:00:00 2001
From: Christophe Guillon <christophe.guillon@inria.fr>
Date: Mon, 1 Jul 2024 15:35:39 +0200
Subject: [PATCH] [Tensor] Disambiguate undefined Tensor dimensions from Scalar
Tensor
Define a Tensor with undefined dims by explicitly setting mSize to 0.
Provide undefined() method which must be used instead of empty()
to test whether a Tensor as associated dimensions and can be used
for forwarding dimensions downward.
This disambiguates from defined scalar Tensor which actually have
empty dimensions, but size of 1.
Note that as soon as a Tensor with undefined dimensions is resized,
it's dimensions are permanently defined (though may still change).
This change solves issues in operators forwardDims() when
input values are scalar, i.e. !undefined() and empty().
---
include/aidge/data/Tensor.hpp | 33 ++++++++++++++-----
include/aidge/operator/Identity.hpp | 2 +-
python_binding/data/pybind_Tensor.cpp | 1 +
src/backend/OperatorImpl.cpp | 6 ++--
src/data/Tensor.cpp | 5 ++-
src/graph/GraphView.cpp | 8 ++---
src/operator/Gather.cpp | 2 +-
src/operator/Memorize.cpp | 6 ++--
src/operator/OperatorTensor.cpp | 6 ++--
src/operator/Reshape.cpp | 2 +-
src/operator/Resize.cpp | 14 ++++----
src/operator/Slice.cpp | 8 ++---
src/operator/Split.cpp | 2 +-
unit_tests/data/Test_Tensor.cpp | 2 +-
.../operator/Test_GlobalAveragePooling_Op.cpp | 4 +--
15 files changed, 58 insertions(+), 43 deletions(-)
diff --git a/include/aidge/data/Tensor.hpp b/include/aidge/data/Tensor.hpp
index ffee8c41a..108f1f2b4 100644
--- a/include/aidge/data/Tensor.hpp
+++ b/include/aidge/data/Tensor.hpp
@@ -57,7 +57,8 @@ class Tensor : public Data,
/**
* @brief Construct a new empty Tensor object.
- * It has the features of an undefined scalar.
+ * It is considered undefined, i.e. dims can't be forwarded from such a Tensor.
+ * @ref undefined() method for details
*/
Tensor(DataType dtype = DataType::Float32, DataFormat dformat = DataFormat::Default)
: Data(Type),
@@ -65,7 +66,7 @@ class Tensor : public Data,
mDataFormat(dformat),
mDims(std::vector<DimSize_t>({})),
mStrides({1}),
- mSize(1)
+ mSize(0)
{
// ctor
}
@@ -523,14 +524,30 @@ public:
void resize(const std::vector<DimSize_t> &dims, std::vector<DimSize_t> strides = std::vector<DimSize_t>());
/**
- * @brief Return if the Tensor object has at leastone element.
- * @return true
- * @return false
+ * @brief Return whether the Tensor object as a rank of 0, i.e. dimensions == {}.
+ * For defined Tensors, this implies that the Tensor is scalar.
+ * For backward compatibility reasons, it is valid to call this predicate
+ * even on undefined Tensors, in which case it returns true.
+ * Hence before test the rank with this method, always check that the
+ * Tensor is not undefined().
+ * In particular for operations such as forwardDims(), one should always
+ * use undefined() to test whether the Tensor dimensions have been defined.
+ * In this case empty() can be used to distinguish scalars from N-D Tensors.
+ * @return true if rank is 0 or the tensor is undefined
*/
bool empty() const { return mDims.empty(); }
- // bool newempty() const noexcept {
- // return mSize == 0;
- // }
+
+ /**
+ * @brief Returns whether the Tensor object is undefined.
+ * An undefined Tensor is equivalent to a tensor for which dimensions have not
+ * been defined yet. Hence, dimensions forwarding can't be done from undefined tensors.
+ * The only cases where a tensor is undefined is after the default constructor
+ * and before any call to resize().
+ * Also, as soon as the resize() method has been called, the Tensor is irreversibly defined.
+ * @ref empty() method for distinguishing an undefined from a scalar
+ * @return true if undefined
+ */
+ bool undefined() const { return mSize == 0; }
/**
* @brief Set each element of the tensor to zero.
diff --git a/include/aidge/operator/Identity.hpp b/include/aidge/operator/Identity.hpp
index 393798da2..e07df59d8 100644
--- a/include/aidge/operator/Identity.hpp
+++ b/include/aidge/operator/Identity.hpp
@@ -76,7 +76,7 @@ public:
* @return false Input has no dimensions or is a nullptr.
*/
bool dimsForwarded() const override final {
- return mInputs[0] ? (mInputs[0]->empty() ? false : mInputs[0]->dims() == mOutputs[0]->dims()) : false;
+ return mInputs[0] ? (mInputs[0]->undefined() ? false : mInputs[0]->dims() == mOutputs[0]->dims()) : false;
}
diff --git a/python_binding/data/pybind_Tensor.cpp b/python_binding/data/pybind_Tensor.cpp
index 60283039b..1d0f02a50 100644
--- a/python_binding/data/pybind_Tensor.cpp
+++ b/python_binding/data/pybind_Tensor.cpp
@@ -93,6 +93,7 @@ void init_Tensor(py::module& m){
.def("get_coord", &Tensor::getCoord)
.def("get_idx", &Tensor::getIdx)
.def_static("get_available_backends", &Tensor::getAvailableBackends)
+ .def("undefined", &Tensor::undefined)
.def("__str__", [](Tensor& b) {
if (b.empty()) {
return std::string("{}");
diff --git a/src/backend/OperatorImpl.cpp b/src/backend/OperatorImpl.cpp
index de200300a..d992703fe 100644
--- a/src/backend/OperatorImpl.cpp
+++ b/src/backend/OperatorImpl.cpp
@@ -29,7 +29,7 @@ Aidge::OperatorImpl::OperatorImpl(const Operator& op, const std::string& backend
Aidge::Elts_t Aidge::OperatorImpl::getNbRequiredData(const Aidge::IOIndex_t inputIdx) const {
if (mOp.getRawInput(inputIdx)) {
const auto input = std::static_pointer_cast<Tensor>(mOp.getRawInput(inputIdx));
- if (!input->empty()) {
+ if (!input->undefined()) {
// Known amount of data: requires the whole tensor by default
return Elts_t::DataElts(input->size());
}
@@ -46,7 +46,7 @@ Aidge::Elts_t Aidge::OperatorImpl::getNbRequiredData(const Aidge::IOIndex_t inpu
Aidge::Elts_t Aidge::OperatorImpl::getNbRequiredProtected(IOIndex_t inputIdx) const {
if (mOp.getRawInput(inputIdx)) {
const auto input = std::static_pointer_cast<Tensor>(mOp.getRawInput(inputIdx));
- if (!input->empty()) {
+ if (!input->undefined()) {
// Known amount of data: protect the whole tensor by default
return Elts_t::DataElts(input->size());
}
@@ -67,7 +67,7 @@ Aidge::Elts_t Aidge::OperatorImpl::getRequiredMemory(const Aidge::IOIndex_t outp
const std::vector<Aidge::DimSize_t> &/*inputsSize*/) const {
if (mOp.getRawOutput(outputIdx)) {
const auto output = std::static_pointer_cast<Tensor>(mOp.getRawOutput(outputIdx));
- if (!output->empty()) {
+ if (!output->undefined()) {
// Known amount of data: requires the whole tensor by default,
// regardless of available data on inputs
return Elts_t::DataElts(output->size());
diff --git a/src/data/Tensor.cpp b/src/data/Tensor.cpp
index 28fb90ceb..d1bf32594 100644
--- a/src/data/Tensor.cpp
+++ b/src/data/Tensor.cpp
@@ -150,13 +150,12 @@ Aidge::Tensor::~Tensor() noexcept = default;
void Aidge::Tensor::resize(const std::vector<Aidge::DimSize_t>& dims,
std::vector<Aidge::DimSize_t> strides) {
- // TODO: scalar Tensor not handled
if (dims.empty()) { // scalar
mDims = std::vector<DimSize_t>(0);
mStrides = std::vector<DimSize_t>({1});
mContiguous = true;
- computeSize();
+ computeSize(); // will set mSize to 1
if (mImpl) {
mImpl->resize(mDims);
}
@@ -214,7 +213,7 @@ void Aidge::Tensor::resize(const std::vector<Aidge::DimSize_t>& dims,
std::string Aidge::Tensor::toString() const {
AIDGE_ASSERT(
- mImpl && (dims().empty() || (dims() == std::vector<DimSize_t>({0})) ||
+ mImpl && (undefined() || (dims() == std::vector<DimSize_t>({0})) ||
(mImpl->hostPtr() != nullptr)),
"tensor should have a valid host pointer");
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index 9528e511b..4ec333445 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -152,7 +152,7 @@ void Aidge::GraphView::save(const std::string& path, bool verbose, bool showProd
// Add-on to display the operator's output dimensions
std::string dims = "";
const auto op = std::dynamic_pointer_cast<OperatorTensor>(node_ptr->getOperator());
- if (op && !op->getOutput(outputIdx)->dims().empty()) {
+ if (op && !op->getOutput(outputIdx)->undefined()) {
dims += " " + fmt::format("{}", op->getOutput(outputIdx)->dims());
}
@@ -198,7 +198,7 @@ void Aidge::GraphView::save(const std::string& path, bool verbose, bool showProd
// Add-on to display the operator's output dimensions
std::string dims = "";
const auto op = std::dynamic_pointer_cast<OperatorTensor>(output.first->getOperator());
- if (op && op->getOutput(output.second) && !op->getOutput(output.second)->dims().empty()) {
+ if (op && op->getOutput(output.second) && !op->getOutput(output.second)->undefined()) {
dims += " " + fmt::format("{}", op->getOutput(output.second)->dims());
}
@@ -441,8 +441,8 @@ bool Aidge::GraphView::forwardDims(const std::vector<std::vector<Aidge::DimSize_
// Input is missing
AIDGE_ASSERT(nodePtr->getOperator()->getRawInput(i),
"Missing input#{} for node {} ({})", i, nodePtr->name(), nodePtr->type());
- AIDGE_ASSERT(!std::static_pointer_cast<Tensor>(nodePtr->getOperator()->getRawInput(i))->empty(),
- "Empty input#{} for node {} ({})", i, nodePtr->name(), nodePtr->type());
+ AIDGE_ASSERT(!std::static_pointer_cast<Tensor>(nodePtr->getOperator()->getRawInput(i))->undefined(),
+ "Undefined input#{} for node {} ({})", i, nodePtr->name(), nodePtr->type());
}
}
diff --git a/src/operator/Gather.cpp b/src/operator/Gather.cpp
index c28a0587a..cd3c43574 100644
--- a/src/operator/Gather.cpp
+++ b/src/operator/Gather.cpp
@@ -51,7 +51,7 @@ void Aidge::Gather_OpImpl::forward() {
const std::string Aidge::Gather_Op::Type = "Gather";
bool Aidge::Gather_Op::dimsForwarded() const {
- if (getInput(1) && !getInput(1)->empty()) {
+ if (getInput(1) && !getInput(1)->undefined()) {
// output dims are data dependent
return false;
}
diff --git a/src/operator/Memorize.cpp b/src/operator/Memorize.cpp
index adf79b5c6..88a182f2a 100644
--- a/src/operator/Memorize.cpp
+++ b/src/operator/Memorize.cpp
@@ -85,12 +85,12 @@ bool Aidge::Memorize_Op::forwardDims(bool /*allowDataDependency*/) {
if (inputsAssociated(false)) {
// Only require one of the input to have dims defined
// Otherwise, forwardDims() won't converge!
- if (!(getInput(0)->empty())) {
+ if (!(getInput(0)->undefined())) {
const auto expectedDims = getInput(0)->dims();
mOutputs[0]->resize(expectedDims);
return true;
}
- else if (!(getInput(1)->empty())) {
+ else if (!(getInput(1)->undefined())) {
const auto expectedDims = getInput(1)->dims();
mOutputs[0]->resize(expectedDims);
return true;
@@ -105,7 +105,7 @@ bool Aidge::Memorize_Op::dimsForwarded() const {
bool forwarded = true;
// check outputs have been filled
for (IOIndex_t i = 0; i < nbOutputs(); ++i) {
- forwarded &= !(getOutput(i)->empty());
+ forwarded &= !(getOutput(i)->undefined());
}
return forwarded;
}
diff --git a/src/operator/OperatorTensor.cpp b/src/operator/OperatorTensor.cpp
index 5df90020a..938a386c4 100644
--- a/src/operator/OperatorTensor.cpp
+++ b/src/operator/OperatorTensor.cpp
@@ -123,7 +123,7 @@ bool Aidge::OperatorTensor::inputsAssociated(bool checkNonEmpty) const {
}
if (checkNonEmpty && getInput(i)) {
- associated &= !(getInput(i)->empty());
+ associated &= !(getInput(i)->undefined());
}
}
@@ -152,13 +152,13 @@ bool Aidge::OperatorTensor::dimsForwarded() const {
// check both inputs and outputs have been filled
for (IOIndex_t i = 0; i < nbInputs(); ++i) {
if (inputCategory(i) != InputCategory::OptionalData && inputCategory(i) != InputCategory::OptionalParam) {
- forwarded &= mInputs[i] ? !(getInput(i)->empty()) : false;
+ forwarded &= mInputs[i] ? !(getInput(i)->undefined()) : false;
}
}
for (IOIndex_t i = 0; i < nbOutputs(); ++i) {
// If getOutput(i) is nullptr, ignore this output (it may be a dummy
// output in a MetaOperator)
- forwarded &= (getOutput(i)) ? !(getOutput(i)->empty()) : true;
+ forwarded &= (getOutput(i)) ? !(getOutput(i)->undefined()) : true;
}
return forwarded;
}
diff --git a/src/operator/Reshape.cpp b/src/operator/Reshape.cpp
index 4184fc18a..cc31eeea7 100644
--- a/src/operator/Reshape.cpp
+++ b/src/operator/Reshape.cpp
@@ -31,7 +31,7 @@ void Aidge::Reshape_OpImpl::forward() {
const std::string Aidge::Reshape_Op::Type = "Reshape";
bool Aidge::Reshape_Op::dimsForwarded() const {
- if (getInput(1) && !getInput(1)->empty()) {
+ if (getInput(1) && !getInput(1)->undefined()) {
// output dims are data dependent
return false;
}
diff --git a/src/operator/Resize.cpp b/src/operator/Resize.cpp
index 966e1c3e0..0d407d4f9 100644
--- a/src/operator/Resize.cpp
+++ b/src/operator/Resize.cpp
@@ -27,9 +27,9 @@ const std::string Aidge::Resize_Op::Type = "Resize";
bool Aidge::Resize_Op::dimsForwarded() const {
// in case of ROI add getInput(1) condition
- if ((getInput(1) && !getInput(1)->empty())
- || (getInput(2) && !getInput(2)->empty())
- || (getInput(3) && !getInput(3)->empty())
+ if ((getInput(1) && !getInput(1)->undefined())
+ || (getInput(2) && !getInput(2)->undefined())
+ || (getInput(3) && !getInput(3)->undefined())
)
{
// output dims are data dependent
@@ -44,9 +44,9 @@ bool Aidge::Resize_Op::forwardDims(bool allowDataDependency) {
AIDGE_ASSERT(getInput(0)->nbDims() == 4,
"input tensor must have dimensions = 4 (batch, channel, height, width).");
- const bool input1ROIPresent = getInput(1) && !getInput(1)->empty();
- const bool input2ScalesPresent = getInput(2) && !getInput(2)->empty();
- const bool input3SizesPresent = getInput(3) && !getInput(3)->empty();
+ const bool input1ROIPresent = getInput(1) && !getInput(1)->undefined();
+ const bool input2ScalesPresent = getInput(2) && !getInput(2)->undefined();
+ const bool input3SizesPresent = getInput(3) && !getInput(3)->undefined();
AIDGE_ASSERT(input2ScalesPresent != input3SizesPresent, "Only one of scales and sizes can be specified.")
@@ -118,4 +118,4 @@ void Aidge::Resize_Op::setBackend(const std::string& name, Aidge::DeviceIdx_t de
if(getInput(3)) {
getInput(3)->setBackend(name, device);
}
-}
\ No newline at end of file
+}
diff --git a/src/operator/Slice.cpp b/src/operator/Slice.cpp
index 3cc2de686..4fcfd587a 100644
--- a/src/operator/Slice.cpp
+++ b/src/operator/Slice.cpp
@@ -29,10 +29,10 @@
const std::string Aidge::Slice_Op::Type = "Slice";
bool Aidge::Slice_Op::dimsForwarded() const {
- if ((getInput(1) && !getInput(1)->empty())
- || (getInput(2) && !getInput(2)->empty())
- || (getInput(3) && !getInput(3)->empty())
- || (getInput(4) && !getInput(4)->empty()))
+ if ((getInput(1) && !getInput(1)->undefined())
+ || (getInput(2) && !getInput(2)->undefined())
+ || (getInput(3) && !getInput(3)->undefined())
+ || (getInput(4) && !getInput(4)->undefined()))
{
// output dims are data dependent
return false;
diff --git a/src/operator/Split.cpp b/src/operator/Split.cpp
index a0cb049b1..31de75e41 100644
--- a/src/operator/Split.cpp
+++ b/src/operator/Split.cpp
@@ -55,7 +55,7 @@ void Aidge::Split_OpImpl::forward() {
const std::string Aidge::Split_Op::Type = "Split";
bool Aidge::Split_Op::dimsForwarded() const {
- if ((getInput(1) && !getInput(1)->empty()))
+ if ((getInput(1) && !getInput(1)->undefined()))
{
// output dims are data dependent
return false;
diff --git a/unit_tests/data/Test_Tensor.cpp b/unit_tests/data/Test_Tensor.cpp
index 62e90dcbd..98d3193ff 100644
--- a/unit_tests/data/Test_Tensor.cpp
+++ b/unit_tests/data/Test_Tensor.cpp
@@ -36,7 +36,7 @@ TEST_CASE("[core/data] Tensor(Construction)", "[Tensor][Constructor]") {
Tensor T_default{};
REQUIRE((
(T_default.dataType() == DataType::Float32) &&
- (T_default.size() == 1) &&
+ (T_default.size() == 0) &&
(T_default.dims() == std::vector<DimSize_t>({})) &&
(T_default.strides() == std::vector<DimSize_t>({1})) &&
(T_default.getImpl() == nullptr) &&
diff --git a/unit_tests/operator/Test_GlobalAveragePooling_Op.cpp b/unit_tests/operator/Test_GlobalAveragePooling_Op.cpp
index d20f689ab..1d99fc7a5 100644
--- a/unit_tests/operator/Test_GlobalAveragePooling_Op.cpp
+++ b/unit_tests/operator/Test_GlobalAveragePooling_Op.cpp
@@ -46,9 +46,7 @@ TEST_CASE("[core/operator] GlobalAveragePooling_Op(forwardDims)",
SECTION("Connected Inputs") {
SECTION("empty tensor") {
for (uint16_t trial = 0; trial < NB_TRIALS; ++trial) {
- const std::size_t nb_dims = 0;
- std::vector<std::size_t> dims(nb_dims);
- input_T->resize(dims);
+ // Test that on undefined input it does not fail
REQUIRE_NOTHROW(op->forwardDims());
}
}
--
GitLab