Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tpetra: Fix #4627 #5315

Merged
merged 1 commit into from
Jun 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,30 @@ class BlockMultiVector :
//! \name Constructors
//@{

/// \brief Default constructor.
///
/// Creates an empty BlockMultiVector. An empty BlockMultiVector
/// has zero rows, and block size zero.
BlockMultiVector ();

//! Copy constructor (shallow copy).
BlockMultiVector (const BlockMultiVector<Scalar, LO, GO, Node>&) = default;

//! Move constructor (shallow move).
BlockMultiVector (BlockMultiVector<Scalar, LO, GO, Node>&&) = default;

//! Copy assigment (shallow copy).
BlockMultiVector<Scalar, LO, GO, Node>&
operator= (const BlockMultiVector<Scalar, LO, GO, Node>&) = default;

//! Move assigment (shallow move).
BlockMultiVector<Scalar, LO, GO, Node>&
operator= (BlockMultiVector<Scalar, LO, GO, Node>&&) = default;

//! "Copy constructor" with option to deep copy.
BlockMultiVector (const BlockMultiVector<Scalar, LO, GO, Node>& in,
const Teuchos::DataAccess copyOrView);

/// \brief Constructor that takes a mesh Map, a block size, and a
/// number of vectors (columns).
///
Expand Down Expand Up @@ -293,12 +317,6 @@ class BlockMultiVector :
const map_type& newMeshMap,
const size_t offset = 0);

/// \brief Default constructor.
///
/// Creates an empty BlockMultiVector. An empty BlockMultiVector
/// has zero rows, and block size zero.
BlockMultiVector ();

//@}
//! \name Access to Maps, the block size, and a MultiVector view.
//@{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,24 @@ getBlockMultiVectorFromSrcDistObject (const Tpetra::SrcDistObject& src)
typedef BlockMultiVector<Scalar, LO, GO, Node> BMV;
const BMV* src_bmv = dynamic_cast<const BMV*> (&src);
TEUCHOS_TEST_FOR_EXCEPTION(
src_bmv == NULL, std::invalid_argument, "Tpetra::Experimental::"
src_bmv == nullptr, std::invalid_argument, "Tpetra::Experimental::"
"BlockMultiVector: The source object of an Import or Export to a "
"BlockMultiVector, must also be a BlockMultiVector.");
return Teuchos::rcp (src_bmv, false); // nonowning RCP
}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
BlockMultiVector (const BlockMultiVector<Scalar, LO, GO, Node>& in,
const Teuchos::DataAccess copyOrView) :
dist_object_type (in),
meshMap_ (in.meshMap_),
pointMap_ (in.pointMap_),
mv_ (in.mv_, copyOrView),
mvData_ (getRawHostPtrFromMultiVector (mv_)),
blockSize_ (in.blockSize_)
{}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
BlockMultiVector (const map_type& meshMap,
Expand All @@ -132,10 +144,7 @@ BlockMultiVector (const map_type& meshMap,
mv_ (Teuchos::rcpFromRef (pointMap_), numVecs), // nonowning RCP is OK, since pointMap_ won't go away
mvData_ (getRawHostPtrFromMultiVector (mv_)),
blockSize_ (blockSize)
{
// Make sure that mv_ has view semantics.
mv_.setCopyOrView (Teuchos::View);
}
{}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
Expand All @@ -149,10 +158,7 @@ BlockMultiVector (const map_type& meshMap,
mv_ (Teuchos::rcpFromRef (pointMap_), numVecs),
mvData_ (getRawHostPtrFromMultiVector (mv_)),
blockSize_ (blockSize)
{
// Make sure that mv_ has view semantics.
mv_.setCopyOrView (Teuchos::View);
}
{}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
Expand All @@ -161,7 +167,7 @@ BlockMultiVector (const mv_type& X_mv,
const LO blockSize) :
dist_object_type (Teuchos::rcp (new map_type (meshMap))), // shallow copy
meshMap_ (meshMap),
mvData_ (NULL), // just for now
mvData_ (nullptr), // just for now
blockSize_ (blockSize)
{
using Teuchos::RCP;
Expand Down Expand Up @@ -195,7 +201,6 @@ BlockMultiVector (const mv_type& X_mv,
// should be marked const anyway, because views can't change the
// allocation (just the entries themselves).
RCP<mv_type> X_view = Teuchos::rcp_const_cast<mv_type> (X_view_const);
X_view->setCopyOrView (Teuchos::View);
TEUCHOS_TEST_FOR_EXCEPTION(
X_view->getCopyOrView () != Teuchos::View, std::logic_error, "Tpetra::"
"Experimental::BlockMultiVector constructor: We just set a MultiVector "
Expand Down Expand Up @@ -225,10 +230,7 @@ BlockMultiVector (const BlockMultiVector<Scalar, LO, GO, Node>& X,
mv_ (X.mv_, newPointMap, offset * X.getBlockSize ()), // MV "offset view" constructor
mvData_ (getRawHostPtrFromMultiVector (mv_)),
blockSize_ (X.getBlockSize ())
{
// Make sure that mv_ has view semantics.
mv_.setCopyOrView (Teuchos::View);
}
{}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
Expand All @@ -241,21 +243,15 @@ BlockMultiVector (const BlockMultiVector<Scalar, LO, GO, Node>& X,
mv_ (X.mv_, pointMap_, offset * X.getBlockSize ()), // MV "offset view" constructor
mvData_ (getRawHostPtrFromMultiVector (mv_)),
blockSize_ (X.getBlockSize ())
{
// Make sure that mv_ has view semantics.
mv_.setCopyOrView (Teuchos::View);
}
{}

template<class Scalar, class LO, class GO, class Node>
BlockMultiVector<Scalar, LO, GO, Node>::
BlockMultiVector () :
dist_object_type (Teuchos::null),
mvData_ (NULL),
mvData_ (nullptr),
blockSize_ (0)
{
// Make sure that mv_ has view semantics.
mv_.setCopyOrView (Teuchos::View);
}
{}

template<class Scalar, class LO, class GO, class Node>
typename BlockMultiVector<Scalar, LO, GO, Node>::map_type
Expand Down Expand Up @@ -463,9 +459,9 @@ getMultiVectorFromSrcDistObject (const Tpetra::SrcDistObject& src)
// the Import or Export calls checkSizes in DistObject's doTransfer.
typedef BlockMultiVector<Scalar, LO, GO, Node> this_type;
const this_type* srcBlkVec = dynamic_cast<const this_type*> (&src);
if (srcBlkVec == NULL) {
if (srcBlkVec == nullptr) {
const mv_type* srcMultiVec = dynamic_cast<const mv_type*> (&src);
if (srcMultiVec == NULL) {
if (srcMultiVec == nullptr) {
// FIXME (mfh 05 May 2014) Tpetra::MultiVector's operator=
// currently does a shallow copy by default. This is why we
// return by RCP, rather than by value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,30 @@ class BlockVector : public BlockMultiVector<Scalar, LO, GO, Node> {
//! \name Constructors
//@{

/// \brief Default constructor.
///
/// Creates an empty BlockVector. An empty BlockVector has zero
/// rows, and block size zero.
BlockVector ();

//! Copy constructor (shallow copy).
BlockVector (const BlockVector<Scalar, LO, GO, Node>&) = default;

//! Move constructor (shallow move).
BlockVector (BlockVector<Scalar, LO, GO, Node>&&) = default;

//! Copy assigment (shallow copy).
BlockVector<Scalar, LO, GO, Node>&
operator= (const BlockVector<Scalar, LO, GO, Node>&) = default;

//! Move assigment (shallow move).
BlockVector<Scalar, LO, GO, Node>&
operator= (BlockVector<Scalar, LO, GO, Node>&&) = default;

//! "Copy constructor" with option to deep copy.
BlockVector (const BlockVector<Scalar, LO, GO, Node>& in,
const Teuchos::DataAccess copyOrView);

/// \brief Constructor that takes a mesh Map and a block size.
///
/// \param meshMap [in] Map that describes the distribution of mesh
Expand Down Expand Up @@ -230,12 +254,6 @@ class BlockVector : public BlockMultiVector<Scalar, LO, GO, Node> {
const map_type& newMeshMap,
const size_t offset = 0);

/// \brief Default constructor.
///
/// Creates an empty BlockVector. An empty BlockVector has zero
/// rows, and block size zero.
BlockVector ();

//@}
//! \name Access to Maps, the block size, and a Vector view.
//@{
Expand Down
33 changes: 15 additions & 18 deletions packages/tpetra/core/src/Tpetra_Experimental_BlockVector_def.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
namespace Tpetra {
namespace Experimental {

template<class Scalar, class LO, class GO, class Node>
BlockVector<Scalar, LO, GO, Node>::
BlockVector () :
base_type ()
{}

template<class Scalar, class LO, class GO, class Node>
BlockVector<Scalar, LO, GO, Node>::
BlockVector (const BlockVector<Scalar, LO, GO, Node>& in,
const Teuchos::DataAccess copyOrView) :
base_type (in, copyOrView)
{}

template<class Scalar, class LO, class GO, class Node>
BlockVector<Scalar, LO, GO, Node>::
BlockVector (const map_type& meshMap, const LO blockSize) :
Expand Down Expand Up @@ -97,15 +110,10 @@ namespace Experimental {
base_type (X, newMeshMap, offset)
{}

template<class Scalar, class LO, class GO, class Node>
BlockVector<Scalar, LO, GO, Node>::
BlockVector () : base_type () {}

template<class Scalar, class LO, class GO, class Node>
typename BlockVector<Scalar, LO, GO, Node>::vec_type
BlockVector<Scalar, LO, GO, Node>::getVectorView () {
Teuchos::RCP<vec_type> vPtr = this->mv_.getVectorNonConst (0);
vPtr->setCopyOrView (Teuchos::View);
return *vPtr; // shallow copy
}

Expand All @@ -123,7 +131,6 @@ namespace Experimental {
return ((const base_type*) this)->replaceGlobalValues (globalRowIndex, 0, vals);
}


template<class Scalar, class LO, class GO, class Node>
bool
BlockVector<Scalar, LO, GO, Node>::
Expand Down Expand Up @@ -152,25 +159,15 @@ namespace Experimental {
return ((const base_type*) this)->getGlobalRowView (globalRowIndex, 0, vals);
}

/// \brief Get a view of the degrees of freedom at the given mesh point.
///
/// \warning This method's interface may change or disappear at any
/// time. Please do not rely on it in your code yet.
///
/// The preferred way to refer to little_vec_type is to get it from
/// BlockVector's typedef. This is because different
/// specializations of BlockVector reserve the right to use
/// different types to implement little_vec_type. This gives us a
/// porting strategy to move from "classic" Tpetra to the Kokkos
/// refactor version.
template<class Scalar, class LO, class GO, class Node>
typename BlockVector<Scalar, LO, GO, Node>::little_vec_type
BlockVector<Scalar, LO, GO, Node>::
getLocalBlock (const LO localRowIndex) const
{
if (! this->isValidLocalMeshIndex (localRowIndex)) {
return little_vec_type ();
} else {
}
else {
const size_t blockSize = this->getBlockSize ();
const size_t offset = localRowIndex * blockSize;
return little_vec_type (this->getRawPtr () + offset, blockSize);
Expand Down
64 changes: 64 additions & 0 deletions packages/tpetra/core/test/Block/BlockMultiVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,38 @@ namespace {
map_type pointMap = BMV::makePointMap (meshMap, blockSize);
TEST_ASSERT( pointMap.isSameAs (X.getPointMap ()) );

// Test BlockMultiVector's two-argument "copy constructor" that
// can make either a deep or a shallow copy.
{
BMV X2 (X, Teuchos::Copy);
auto X2_map = X2.getMap ();
const bool maps_ok = ! X2_map.is_null () &&
! X.getMap ().is_null () &
X2_map->isSameAs (* (X.getMap ()));
TEST_ASSERT( maps_ok );

auto X_mv_h = X.getMultiVectorView ().getLocalViewHost ();
auto X2_mv_h = X2.getMultiVectorView ().getLocalViewHost ();
TEST_ASSERT( X_mv_h.extent (0) == X2_mv_h.extent (0) &&
X_mv_h.extent (1) == X2_mv_h.extent (1) &&
X_mv_h.data () != X2_mv_h.data () );
}

{
BMV X2 (X, Teuchos::View);
auto X2_map = X2.getMap ();
const bool maps_ok = ! X2_map.is_null () &&
! X.getMap ().is_null () &
X2_map->isSameAs (* (X.getMap ()));
TEST_ASSERT( maps_ok );

auto X_mv_h = X.getMultiVectorView ().getLocalViewHost ();
auto X2_mv_h = X2.getMultiVectorView ().getLocalViewHost ();
TEST_ASSERT( X_mv_h.extent (0) == X2_mv_h.extent (0) &&
X_mv_h.extent (1) == X2_mv_h.extent (1) &&
X_mv_h.data () == X2_mv_h.data () );
}

// Exercise the four-argument constructor (that uses an existing
// point Map).
BMV Y (meshMap, pointMap, blockSize, numVecs);
Expand Down Expand Up @@ -173,6 +205,38 @@ namespace {
V2.getLocalViewHost ().data () );
TEST_EQUALITY( V1->getLocalViewDevice ().data (),
V2.getLocalViewDevice ().data () );

// Test BlockVector's two-argument "copy constructor" that can
// make either a deep or a shallow copy.
{
BV V_a (V, Teuchos::Copy);
auto V_a_map = V_a.getMap ();
const bool maps_ok = ! V_a_map.is_null () &&
! V.getMap ().is_null () &
V_a_map->isSameAs (* (V.getMap ()));
TEST_ASSERT( maps_ok );

auto V_v_h = V.getVectorView ().getLocalViewHost ();
auto V_a_v_h = V_a.getVectorView ().getLocalViewHost ();
TEST_ASSERT( V_v_h.extent (0) == V_a_v_h.extent (0) &&
V_v_h.extent (1) == V_a_v_h.extent (1) &&
V_v_h.data () != V_a_v_h.data () );
}

{
BV V_a (V, Teuchos::View);
auto V_a_map = V_a.getMap ();
const bool maps_ok = ! V_a_map.is_null () &&
! V.getMap ().is_null () &
V_a_map->isSameAs (* (V.getMap ()));
TEST_ASSERT( maps_ok );

auto V_v_h = V.getVectorView ().getLocalViewHost ();
auto V_a_v_h = V_a.getVectorView ().getLocalViewHost ();
TEST_ASSERT( V_v_h.extent (0) == V_a_v_h.extent (0) &&
V_v_h.extent (1) == V_a_v_h.extent (1) &&
V_v_h.data () == V_a_v_h.data () );
}
}

// Test BlockMultiVector::getMultiVectorView. It must return a
Expand Down