Document and add debug-mode runtime checks non-aliasing assumptions
Created by: bartlettroscoe
CC: @trilinos/thyra
Description:
During the refactoring work for Thyra in #330, it was discovered that Belos aliases some of the input and output arguments in calls the Thyra::linear_combination(). That complicated the refactoring done in #330. The assumption in Thyra from the earliest days (when it was called TSFCore, see TSFCoreSAND.pdf) was that input objects must not alias output arguments, or the behavior is undefined (just like with Fortran). However, some implementations of operations (like the element-wise implementation of Thyra::linear_combination() using the RTOp::LinearCombinationTOp subclass) allowed aliasing and worked correctly (by accident).
The reason that Thyra has the non-aliasing assumption is for just the reason for the complexity that had to be added in the commit 1de126c8 . It is a pain to write the implementations of things if you have to be constantly checking for aliasing and it creates inefficeint code that has to create expensive temp copies in order to carry out requested operations (like in the Belos example). And with abstract objects, it can be very hard to detect aliasing of underlying memory (I can give several examples of this). So the best policy is to just strictly disallow aliasing and say that if clients pass in aliased input and output objects, then the results are undefined. That is what the Fortran standard does for the entire language (and since Thyra objects can call Fortran functions, this makes sense as well).
This story is to:
- Add clear documentation (both at the global level and the prototype for each function) that aliasing is not allowed (and if inputs and outputs are aliased, the results are undefined).
- Add debug-mode runtime checks for aliasing of input and output arguments to all of the non-virtual interface (NIV) member functions in all of the Thyra classes as possible.
- Update all of the Trilinos and customer code as possible that is passing in aliased input and output arguments.
- Remove current Thyra implementions that try to handle aliasing.
For all other external customers of Thyra, they will need to fix their code to not pass in aliased input and output objects (and create the temp objects themselves if needed).
Possible Implementation:
The way to implement this for Thyra is to add a new member function aliasStatus(mv)
to the Thyra::LinearOpBase interface:
namespace Thyra {
// We can use C++11 now due to #1465!
enum class EAliasStatus { IS_ALIASED, NOT_ALIASED, UNKNOWN };
template<class Scalar>
class LinearOpBase: ... {
private:
...
// NVI public function
EAliasStatus aliasStatus(const MultiVectorBase<Scalar> &mv) const
{ return this->aliasStatusImpl(mv); }
...
protected:
...
// NVI virtual function
virtual EAliasStatus aliasStatus(const MultiVectorBase<Scalar> &mv) const = 0;
...
};
} // namespace Thyra
The Thyra::LinearOpDefaultBase class would have a default override of aliasStatus()
that returned Thyra::EAliasStatus::UNKNOWN
(so this would not break backward compatibility).
Thyra would then provide a non-member non-friend helper function like:
template<class Scalar>
void debugAssertNonAliasedInputsOutputs(
const ArrayView<const Ptr<const LinearOpBase<Scalar> > > &inputs,
const ArrayView<const Ptr<const LinearOpBase<Scalar> > > &outputs,
const std::string &functionName );
That function would then throw an exception if it detected that any of the input[i]
objects aliased any of the output[j]
objects (using the new aliasStatus()
member function).
Since every Thyra::LinearOpBase, Thyra::MultiVectorBase, and Thyra::VectorBase object all derive from Thyra::LinearOpBase, this one function could be used to assert non-aliasing in every operation in all of the Thyra interfaces like, for example:
template<class Scalar>
class LinearOpBase ... {
...
void apply(
const EOpTransp M_trans,
const MultiVectorBase< Scalar > &X,
const Ptr< MultiVectorBase< Scalar > > &Y,
const Scalar alpha = static_cast< Scalar >(1.0),
const Scalar beta = static_cast< Scalar >(0.0)
)
{
debugAssertNonAliasedInputsOutputs( Teuchos::tuple<...>(Y),
Teuchos::tuple<...>(Teuchos::ptr(*this), Teuchos::ptrFromRef(X),
"Thyra::LinearOpBase::apply(M_trans, X, Y, alpha, beta)" );
applyImpl(M_trans, X, Y, alpha, beta);
}
...
};
Once the function debugAssertNonAliasedInputsOutputs()
is written and unit tested, it should not take long to add calls to this function (or indirectly using helper macros perhaps) in all of the Thyra intreface functions (that are using the NVI idiom anyway).
The harder part might be fixing Trilinos code (like Belos) trying to alias input and output objects.