Skip to content

Commit

Permalink
Tpetra::{Ex,Im}port: Fix #607 (add isLocallyComplete method)
Browse files Browse the repository at this point in the history 
@trilinos/tpetra Add the proposed isLocallyComplete method (see #607)
to Tpetra::Export and Tpetra::Import.  Also add a test for this
method.  The test passes.
  • Loading branch information
Mark Hoemmen committed Sep 14, 2016
1 parent 5bb9cad commit ce52b64
Show file tree
Hide file tree
Showing 9 changed files with 626 additions and 80 deletions.
24 changes: 21 additions & 3 deletions packages/tpetra/core/src/Tpetra_Details_Transfer_decl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,33 @@ class Transfer : public Teuchos::Describable {
/// process getExportPiDs()[i].
virtual Teuchos::ArrayView<const int> getExportPIDs () const = 0;

//! The source Map used to construct this Export.
//! The source Map used to construct this Export or Import.
virtual Teuchos::RCP<const map_type> getSourceMap () const = 0;

//! The target Map used to construct this Export.
//! The target Map used to construct this Export or Import.
virtual Teuchos::RCP<const map_type> getTargetMap () const = 0;

//! The Distributor that this Export object uses to move data.
//! The Distributor that this Export or Import object uses to move data.
virtual ::Tpetra::Distributor& getDistributor () const = 0;

/// \brief Is this Export or Import locally complete?
///
/// If this is an Export, then do all source Map indices on the
/// calling process exist on at least one process (not necessarily
/// this one) in the target Map?
///
/// If this is an Import, then do all target Map indices on the
/// calling process exist on at least one process (not necessarily
/// this one) in the source Map?
///
/// It's not necessarily an error for an Export or Import not to be
/// locally complete on one or more processes. For example, this
/// may happen in the common use case of "restriction" -- that is,
/// taking a subset of a large object. Nevertheless, you may find
/// this predicate useful for figuring out whether you set up your
/// Maps in the way that you expect.
virtual bool isLocallyComplete () const = 0;

/// \brief Describe this object in a human-readable way to the given
/// output stream.
///
Expand Down
10 changes: 10 additions & 0 deletions packages/tpetra/core/src/Tpetra_Export_decl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,16 @@ namespace Tpetra {
//! The Distributor that this Export object uses to move data.
Distributor & getDistributor() const;

/// \brief Do all source Map indices on the calling process exist
/// on at least one process (not necessarily this one) in the
/// target Map?
///
/// It's not necessarily an error for an Export not to be locally
/// complete on one or more processes. Nevertheless, you may find
/// this predicate useful for figuring out whether you set up your
/// Maps in the way that you expect.
bool isLocallyComplete () const;

//! Assignment operator
Export<LocalOrdinal,GlobalOrdinal,Node>&
operator= (const Export<LocalOrdinal,GlobalOrdinal,Node>& rhs);
Expand Down
40 changes: 34 additions & 6 deletions packages/tpetra/core/src/Tpetra_Export_def.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,12 @@ namespace Tpetra {
return ExportData_->distributor_;
}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
bool
Export<LocalOrdinal,GlobalOrdinal,Node>::isLocallyComplete () const {
return ExportData_->isLocallyComplete_;
}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
Export<LocalOrdinal,GlobalOrdinal,Node>&
Export<LocalOrdinal,GlobalOrdinal,Node>::
Expand Down Expand Up @@ -417,6 +423,8 @@ namespace Tpetra {
typedef LocalOrdinal LO;
typedef GlobalOrdinal GO;
typedef typename ArrayView<const GO>::size_type size_type;
const char tfecfFuncName[] = "setupExport: ";

const map_type& source = * (getSourceMap ());
const map_type& target = * (getTargetMap ());
ArrayView<const GO> sourceGIDs = source.getNodeElementList ();
Expand Down Expand Up @@ -483,12 +491,20 @@ namespace Tpetra {
// overlapping, so multiple processes might send to the same LID
// on a receiving process.

TPETRA_ABUSE_WARNING(
getNumExportIDs() > 0 && ! source.isDistributed(),
std::runtime_error,
"::setupSamePermuteExport(): Source has export LIDs but Source is not "
"distributed globally." << std::endl
<< "Exporting to a submap of the target map.");
if (exportLIDs.size () != 0 && ! source.isDistributed ()) {
// This Export has export LIDs, meaning that the source Map has
// entries on this process that are not in the target Map on
// this process. However, the source Map is not distributed
// globally. This implies that this Import is not locally
// complete on this process.
ExportData_->isLocallyComplete_ = false;
// mfh 12 Sep 2016: I disagree that this is "abuse"; it may be
// correct behavior, depending on the circumstances.
TPETRA_ABUSE_WARNING
(true, std::runtime_error, "::setupSamePermuteExport(): Source has "
"export LIDs but Source is not distributed globally. Exporting to "
"a submap of the target map.");
}

// Compute exportPIDs_ ("outgoing" process IDs).
//
Expand All @@ -509,6 +525,8 @@ namespace Tpetra {
const LookupStatus lookup =
target.getRemoteIndexList (exportGIDs(),
ExportData_->exportPIDs_ ());
// mfh 12 Sep 2016: I disagree that this is "abuse"; it may be
// correct behavior, depending on the circumstances.
TPETRA_ABUSE_WARNING( lookup == IDNotPresent, std::runtime_error,
"::setupSamePermuteExport(): The source Map has GIDs not found "
"in the target Map.");
Expand All @@ -517,10 +535,20 @@ namespace Tpetra {
// exporting to GIDs which don't belong to any process in the
// target Map.
if (lookup == IDNotPresent) {
// There is at least one GID owned by the calling process in
// the source Map, which is not owned by any process in the
// target Map.
ExportData_->isLocallyComplete_ = false;

const size_type numInvalidExports =
std::count_if (ExportData_->exportPIDs_().begin(),
ExportData_->exportPIDs_().end(),
std::bind1st (std::equal_to<int>(), -1));
TEUCHOS_TEST_FOR_EXCEPTION_CLASS_FUNC
(numInvalidExports == 0, std::logic_error, "Calling getRemoteIndexList "
"on the target Map returned IDNotPresent, but none of the returned "
"\"export\" process ranks are -1. Please report this bug to the "
"Tpetra developers.");

// count number of valid and total number of exports
const size_type totalNumExports = ExportData_->exportPIDs_.size();
Expand Down
18 changes: 18 additions & 0 deletions packages/tpetra/core/src/Tpetra_ImportExportData_decl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,24 @@ namespace Tpetra {
/// communicator; it does not complete initialization.
Distributor distributor_;

/// \brief Is this Export or Import locally complete?
///
/// If this is an Export, then do all source Map indices on the
/// calling process exist on at least one process (not necessarily
/// this one) in the target Map?
///
/// If this is an Import, then do all target Map indices on the
/// calling process exist on at least one process (not necessarily
/// this one) in the source Map?
///
/// It's not necessarily an error for an Export or Import not to
/// be locally complete on one or more processes. For example,
/// this may happen in the common use case of "restriction" --
/// that is, taking a subset of a large object. Nevertheless, you
/// may find this predicate useful for figuring out whether you
/// set up your Maps in the way that you expect.
bool isLocallyComplete_;

private:
//! Copy constructor (declared but not defined, do not use)
ImportExportData (const ImportExportData<LocalOrdinal,GlobalOrdinal,Node> &rhs);
Expand Down
21 changes: 17 additions & 4 deletions packages/tpetra/core/src/Tpetra_ImportExportData_def.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ namespace Tpetra {
target_ (target),
out_ (Teuchos::getFancyOStream (Teuchos::rcpFromRef (std::cerr))),
numSameIDs_ (0),
distributor_ (source->getComm (), out_)
distributor_ (source->getComm (), out_),
isLocallyComplete_ (true) // Import/Export constructor may change this
{}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
Expand All @@ -67,7 +68,8 @@ namespace Tpetra {
target_ (target),
out_ (out),
numSameIDs_ (0),
distributor_ (source->getComm (), out_)
distributor_ (source->getComm (), out_),
isLocallyComplete_ (true) // Import/Export constructor may change this
{}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
Expand All @@ -79,7 +81,8 @@ namespace Tpetra {
target_ (target),
out_ (Teuchos::getFancyOStream (Teuchos::rcpFromRef (std::cerr))),
numSameIDs_ (0),
distributor_ (source->getComm (), out_, plist)
distributor_ (source->getComm (), out_, plist),
isLocallyComplete_ (true) // Import/Export constructor may change this
{}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
Expand All @@ -92,7 +95,8 @@ namespace Tpetra {
target_ (target),
out_ (out),
numSameIDs_ (0),
distributor_ (source->getComm (), out_, plist)
distributor_ (source->getComm (), out_, plist),
isLocallyComplete_ (true) // Import/Export constructor may change this
{}

template <class LocalOrdinal, class GlobalOrdinal, class Node>
Expand Down Expand Up @@ -120,13 +124,22 @@ namespace Tpetra {
ArrayView<const int> ProcsFrom = distributor_.getProcsFrom();
ArrayView<const size_t> LengthsFrom = distributor_.getLengthsFrom();

// isLocallyComplete is a local predicate.
// It could be true in one direction but false in another.

bool isLocallyComplete = true; // by default
for (size_t i = 0, j = 0; i < NumReceives; ++i) {
const int pid = ProcsFrom[i];
if (pid == -1) {
isLocallyComplete = false;
}
for (size_t k = 0; k < LengthsFrom[i]; ++k) {
tData->exportPIDs_[j] = pid;
++j;
}
}
tData->isLocallyComplete_ = isLocallyComplete;

return tData;
}

Expand Down
30 changes: 21 additions & 9 deletions packages/tpetra/core/src/Tpetra_Import_decl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ namespace Tpetra {
Import (const Teuchos::RCP<const map_type>& source,
const Teuchos::RCP<const map_type>& target,
Teuchos::Array<int> & remotePIDs);

/// \brief Copy constructor.
///
/// \note Currently this only makes a shallow copy of the Import's
Expand All @@ -228,14 +228,14 @@ namespace Tpetra {
///

Import (const Teuchos::RCP<const Map<LocalOrdinal, GlobalOrdinal, Node> >& source,
const Teuchos::RCP<const Map<LocalOrdinal, GlobalOrdinal, Node> >& target,
Teuchos::Array<int> & userRemotePIDs,
Teuchos::Array<GlobalOrdinal>& remoteGIDs,
const Teuchos::ArrayView<const LocalOrdinal> & userExportLIDs,
const Teuchos::ArrayView<const int> & userExportPIDs,
const bool useRemotePIDs,
const Teuchos::RCP<Teuchos::ParameterList>& plist = Teuchos::null,
const Teuchos::RCP<Teuchos::FancyOStream>& out = Teuchos::null);
const Teuchos::RCP<const Map<LocalOrdinal, GlobalOrdinal, Node> >& target,
Teuchos::Array<int> & userRemotePIDs,
Teuchos::Array<GlobalOrdinal>& remoteGIDs,
const Teuchos::ArrayView<const LocalOrdinal> & userExportLIDs,
const Teuchos::ArrayView<const int> & userExportPIDs,
const bool useRemotePIDs,
const Teuchos::RCP<Teuchos::ParameterList>& plist = Teuchos::null,
const Teuchos::RCP<Teuchos::FancyOStream>& out = Teuchos::null);


//! Destructor.
Expand Down Expand Up @@ -298,6 +298,18 @@ namespace Tpetra {
//! The Distributor that this Import object uses to move data.
Distributor & getDistributor() const;

/// \brief Do all target Map indices on the calling process exist
/// on at least one process (not necessarily this one) in the
/// source Map?
///
/// It's not necessarily an error for an Import not to be locally
/// complete on one or more processes. For example, this may
/// happen in the common use case of "restriction" -- that is,
/// taking a subset of a large object. Nevertheless, you may find
/// this predicate useful for figuring out whether you set up your
/// Maps in the way that you expect.
bool isLocallyComplete () const;

//! Assignment operator.
Import<LocalOrdinal,GlobalOrdinal,Node>&
operator= (const Import<LocalOrdinal,GlobalOrdinal,Node>& Source);
Expand Down
Loading

0 comments on commit ce52b64

Please sign in to comment.