Create Tensor::isScalar() method for better disambiguation

!162 (merged) aimed to disambiguate between empty tensors & scalar tensors. This proposition builds on top of it as we can further simplify the interface.

As of now checking if a tensor is scalar is done as follows : aidge_backend_cpu!75 (diffs).

Unless you take a good look at what happens between Tensor::empty() & Tensor::undefined() it is not easy to understand that we are dealing with a scalar tensor here.

Hence I propose to add the following method to easier code reading

bool Tensor::isScalar() { return empty() && mSize == 1 }

added Discussion 💬 Feature 🚀 labels

changed title from Create Tensor::is_scalar() { return mSize == 1 ; } to Create Tensor::is_scalar() method for better disambiguation

changed title from Create Tensor::is_scalar() method for better disambiguation to Create Tensor::isScalar() method for better disambiguation

changed the description

ping @cguillon @olivierbichler

Agreed that there is something to improve there. We could not change the semantic of empty() and undefined() without a lot of changes elsewhere, and empty() is imho misnamed and probably should not be used anywhere. What is needed I guess generally is:

• is the tensor undefined? undefined() is ok
• what is the rank (i.e. the number of dimensions) of a Tensor:
  • if undefined should be invalid
  • if 0: this is a scalar
  • if > 0: it's not scalar

The problem is we have the interfaces:

• nbDims(): gives the rank, but gives also 0 (a valid rank) when undefined
• empty(): misnamed, returns whether the rank is 0, but also return true when undefined

Thus each time there is one of these calls, there should be first an assert or a test that !undefined().

I would agree to add isScalar(), though for better readability I would implement it as:

bool Tensor::isScalar() { return !undefined() && nbDims() == 0; }

For completion, as scalar is not actually really special (it's just a rank 0 tensor), I would suggest actually to add a rank function returning -1 on undefined:

size_t Tensor::rank() { return !undefined()? nbDims(): -1; }

Leading to:

bool Tensor::isScalar() { return rank() == 0; }

This way, we have two always valid methods:

• rank(), rank and -1 on undefined
• isScalar(), scalar? and false undefined

Related to their "unsafe" counter parts which must be guarded by !undefined:

• nbDims()
• empty() (which actually makes no sense with this name)

These last two may be made obsolete in a second step.

Another option is to modify nbDims() such that it implements rank() (returning -1 on undefined) but it may break elsewhere. And remove empty(), replacing it by isScalar(), also it will require to check all usages.

It's a shorter but more risky route than introducing rank()/isScalar() and latter making nbDims()/empty() obsolete.

I would be in favor of that, I know that if we do it in 2 steps only the 1st step will be done and it will result in a bloated user interface.

Makes sense. Thus:

1. nbDims() will now return -1 on undefined()
2. provide isScalar() as: bool Tensor::isScalar() { return !undefined() && nbDims() == 0; } // but not there that the first term is redundant if nbDims() is implemented as in 1.
3. obsolete empty()

Right?

Looking at it, I do not like much 1. because then nbDims() is inconsistant with Tesnor::dims().size(), and also on the python binding side we only have dims(), hence people may use len(tensor.dims()) anyway.

Then if you do not want to bloat the interface with an additional rank() method, stuck to 2. and 3. and the current assumption that dims()/nbDims() should actually not be used in case of undefined().

What i'm still unsure of is, in term of abstraction, was there a use case for empty() as opposed to undefined(). What would it mean to have an empty() Tensor if it is not equivalent to undefined()?

added UI 🎮 label