From 73f1a423d9a787ca90908afd886a6ddd0c5fe5aa Mon Sep 17 00:00:00 2001 From: Paul Wolfenbarger Date: Mon, 17 Jun 2019 13:45:52 -0600 Subject: [PATCH 1/3] I keep making the mistake of expecting unlink to remove the directory tree. I know better of course and python has done a nice job of complaining about it at least. --- commonTools/framework/clean_workspace/clean_workspace.py | 4 ++-- .../clean_workspace/unittests/test_clean_workspace.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/commonTools/framework/clean_workspace/clean_workspace.py b/commonTools/framework/clean_workspace/clean_workspace.py index 384564b60292..409d4143a438 100755 --- a/commonTools/framework/clean_workspace/clean_workspace.py +++ b/commonTools/framework/clean_workspace/clean_workspace.py @@ -14,7 +14,7 @@ import os import argparse -import subprocess +import shutil from clean_sentinel import clean_reference_date from clean_sentinel import last_clean_date @@ -59,7 +59,7 @@ def force_clean_space(self): """Do the actual cleanup basically just os.unlink() """ - os.unlink(self.args.dir) + shutil.rmtree(self.args.dir) def clean_space_by_date(self): if last_clean_date() < clean_reference_date(): diff --git a/commonTools/framework/clean_workspace/unittests/test_clean_workspace.py b/commonTools/framework/clean_workspace/unittests/test_clean_workspace.py index 8c21666ae509..093ea4e2fd6b 100755 --- a/commonTools/framework/clean_workspace/unittests/test_clean_workspace.py +++ b/commonTools/framework/clean_workspace/unittests/test_clean_workspace.py @@ -120,7 +120,7 @@ def test_calls(self): setattr(test_args, 'force_clean', True) cleanerInst = Cleaner() cleanerInst.args = test_args - with mock.patch('os.unlink') as m_unlink: + with mock.patch('shutil.rmtree') as m_unlink: cleanerInst.force_clean_space() m_unlink.assert_called_once_with(test_args.dir) From 14523e1ab73b7049e73d0e6aa7c4daf9c676ff7c Mon Sep 17 00:00:00 2001 From: Mark Hoemmen Date: Mon, 17 Jun 2019 22:38:00 -0600 Subject: [PATCH 2/3] Tpetra: Add benchmark for CrsMatrix::sumIntoLocalValues --- .../advanced/Benchmarks/CMakeLists.txt | 6 + .../CrsMatrix_sumIntoLocalValues.cpp | 207 ++++++++++++++++++ 2 files changed, 213 insertions(+) create mode 100644 packages/tpetra/core/example/advanced/Benchmarks/CrsMatrix_sumIntoLocalValues.cpp diff --git a/packages/tpetra/core/example/advanced/Benchmarks/CMakeLists.txt b/packages/tpetra/core/example/advanced/Benchmarks/CMakeLists.txt index 04ada91c5e59..cb0fb4b8d688 100644 --- a/packages/tpetra/core/example/advanced/Benchmarks/CMakeLists.txt +++ b/packages/tpetra/core/example/advanced/Benchmarks/CMakeLists.txt @@ -26,3 +26,9 @@ TRIBITS_ADD_EXECUTABLE( SOURCES localView.cpp COMM serial mpi ) + +TRIBITS_ADD_EXECUTABLE( + CrsMatrix_sumIntoLocalValues + SOURCES CrsMatrix_sumIntoLocalValues.cpp + COMM serial mpi +) diff --git a/packages/tpetra/core/example/advanced/Benchmarks/CrsMatrix_sumIntoLocalValues.cpp b/packages/tpetra/core/example/advanced/Benchmarks/CrsMatrix_sumIntoLocalValues.cpp new file mode 100644 index 000000000000..bc9b5b79f20e --- /dev/null +++ b/packages/tpetra/core/example/advanced/Benchmarks/CrsMatrix_sumIntoLocalValues.cpp @@ -0,0 +1,207 @@ +/* +// @HEADER +// *********************************************************************** +// +// Tpetra: Templated Linear Algebra Services Package +// Copyright (2008) Sandia Corporation +// +// Under the terms of Contract DE-AC04-94AL85000 with Sandia Corporation, +// the U.S. Government retains certain rights in this software. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// 1. Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in the +// documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the Corporation nor the names of the +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY SANDIA CORPORATION "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +// PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL SANDIA CORPORATION OR THE +// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +// EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +// PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +// LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +// NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Questions? Contact Michael A. Heroux (maherou@sandia.gov) +// +// ************************************************************************ +// @HEADER +*/ + +#include "Tpetra_CrsMatrix.hpp" +#include "Tpetra_Core.hpp" +#include "Tpetra_Map.hpp" +#include "Tpetra_Details_Profiling.hpp" +#include "Teuchos_CommandLineProcessor.hpp" +#include "Teuchos_CommHelpers.hpp" +#include "Teuchos_TimeMonitor.hpp" +#include +#include +#include + +namespace { // (anonymous) + +using Teuchos::outArg; +using Teuchos::RCP; +using Teuchos::rcp; +using Teuchos::REDUCE_MIN; +using Teuchos::reduceAll; +using TM = Teuchos::TimeMonitor; +using std::endl; +using crs_matrix_type = Tpetra::CrsMatrix<>; +using map_type = Tpetra::Map<>; +using LO = map_type::local_ordinal_type; +using GO = map_type::global_ordinal_type; + +RCP > +createRowAndColMap (RCP > comm, + const LO lclNumInds) +{ + TM mon (*TM::getNewCounter ("createRowAndColMap")); + + const int numProcs = comm->getSize (); + const GO gblNumInds = GO (numProcs) * GO (lclNumInds); + const GO indexBase = 0; + RCP rowAndColMap + (new map_type (gblNumInds, lclNumInds, indexBase, comm)); + return rowAndColMap; +} + +RCP > +createCrsMatrix (RCP rowAndColMap, + const size_t maxNumEntPerRow) +{ + TM mon (*TM::getNewCounter ("CrsMatrix constructor")); + + return rcp (new crs_matrix_type (rowAndColMap, rowAndColMap, + maxNumEntPerRow, + Tpetra::StaticProfile)); +} + +void +populateCrsMatrix (crs_matrix_type& A, + const Tpetra::CrsMatrix<>::local_ordinal_type numToInsert, + const Tpetra::CrsMatrix<>::local_ordinal_type lclColInds[], + const double vals[]) +{ + TM mon (*TM::getNewCounter ("insertLocalValues loop")); + + const LO lclNumRows (A.getNodeNumRows ()); + for (LO lclRow = 0; lclRow < lclNumRows; ++lclRow) { + A.insertLocalValues (lclRow, numToInsert, vals, lclColInds); + } +} + +void +doSumIntoLocalValues (const std::string& label, + crs_matrix_type& A, + const LO numToInsert, + const LO lclColInds[], + const double vals[], + const int numTrials) +{ + TM mon (*TM::getNewCounter (label)); + + const LO lclNumRows (A.getNodeNumRows ()); + + for (int trial = 0; trial < numTrials; ++trial) { + for (LO lclRow = 0; lclRow < lclNumRows; ++lclRow) { + (void) A.sumIntoLocalValues (lclRow, numToInsert, vals, lclColInds); + } + } +} + +struct CmdLineArgs { + int lclNumInds = 10000; + int maxNumEntPerRow = 28; + int numToInsert = 27; + int numTrials = 10; +}; + +void +benchmarkCrsMatrixSumIntoLocalValues (const CmdLineArgs args) +{ + TM mon (*TM::getNewCounter ("Whole benchmark")); + + RCP > comm = Tpetra::getDefaultComm (); + auto rowAndColMap = createRowAndColMap (comm, LO (args.lclNumInds)); + auto A = createCrsMatrix (rowAndColMap, size_t (args.maxNumEntPerRow)); + + std::vector lclColInds (size_t (args.numToInsert)); + std::vector vals (size_t (args.numToInsert), 0.0); + // Skip 0, so that search isn't completely trivial. + std::iota (lclColInds.begin (), lclColInds.end (), LO (1)); + + populateCrsMatrix (*A, LO (args.numToInsert), + lclColInds.data (), vals.data ()); + doSumIntoLocalValues ("sumIntoLocalValues: before first fillComplete", + *A, LO (args.numToInsert), lclColInds.data (), + vals.data (), args.numTrials); + { + TM mon2 (*TM::getNewCounter ("CrsMatrix fillComplete")); + A->fillComplete (); + } + doSumIntoLocalValues ("sumIntoLocalValues: after first fillComplete", + *A, LO (args.numToInsert), lclColInds.data (), + vals.data (), args.numTrials); +} + +} // namespace (anonymous) + +int +main (int argc, char* argv[]) +{ + using Teuchos::CommandLineProcessor; + using std::cerr; + using std::cout; + using std::endl; + + Tpetra::ScopeGuard tpetraScope (&argc, &argv); + CommandLineProcessor cmdp; + CmdLineArgs cmdLineArgs; + cmdp.setOption ("lclNumInds", &cmdLineArgs.lclNumInds, + "Number of rows on each (MPI) process"); + cmdp.setOption ("maxNumEntPerRow", &cmdLineArgs.maxNumEntPerRow, + "Max amount of space to preallocate per row " + "(must be at least as big as numToInsert)"); + cmdp.setOption ("numToInsert", &cmdLineArgs.numToInsert, + "Number of column indices to insert per row"); + cmdp.setOption ("numTrials", &cmdLineArgs.numTrials, + "Number of times to repeat each operation in a " + "timing loop"); + const CommandLineProcessor::EParseCommandLineReturn parseResult = + cmdp.parse (argc, argv); + if (parseResult == CommandLineProcessor::PARSE_HELP_PRINTED) { + // The user specified --help at the command line to print help + // with command-line arguments. We printed help already, so quit + // with a happy return code. + return EXIT_SUCCESS; + } + else if (parseResult != CommandLineProcessor::PARSE_SUCCESSFUL) { + cerr << "Failed to parse command-line arguments." << endl; + return EXIT_FAILURE; + } + cout << "lclNumInds: " << cmdLineArgs.lclNumInds << endl + << "maxNumEntPerRow: " << cmdLineArgs.maxNumEntPerRow << endl + << "numToInsert: " << cmdLineArgs.numToInsert << endl + << "numTrials: " << cmdLineArgs.numTrials << endl; + benchmarkCrsMatrixSumIntoLocalValues (cmdLineArgs); + + auto comm = Tpetra::getDefaultComm (); + TM::report (comm.ptr (), cout); + + return EXIT_SUCCESS; +} From 191c615a5d19313dae7a7430b32f28b6db5f725c Mon Sep 17 00:00:00 2001 From: Mark Hoemmen Date: Tue, 18 Jun 2019 09:57:55 -0600 Subject: [PATCH 3/3] Teuchos::ParameterList: Fix 2 shadowing warnings @trilinos/teuchos --- .../parameterlist/src/Teuchos_ParameterList.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/teuchos/parameterlist/src/Teuchos_ParameterList.cpp b/packages/teuchos/parameterlist/src/Teuchos_ParameterList.cpp index deeb26aa3d1c..1244fded8f9d 100644 --- a/packages/teuchos/parameterlist/src/Teuchos_ParameterList.cpp +++ b/packages/teuchos/parameterlist/src/Teuchos_ParameterList.cpp @@ -516,16 +516,16 @@ void ParameterList::modifyParameterList(ParameterList & valid_pl, ConstIterator itr; for (itr = valid_pl.begin(); itr != valid_pl.end(); ++itr){ const std::string &entry_name = itr->first; - const ParameterEntry &entry = itr->second; - if (entry.isList() && depth > 0){ + const ParameterEntry &cur_entry = itr->second; + if (cur_entry.isList() && depth > 0){ ParameterList &valid_pl_sublist = valid_pl.sublist(entry_name, true); if(!valid_pl_sublist.disableRecursiveModification_){ const ParameterEntry *validEntry = this->getEntryPtr(entry_name); TEUCHOS_TEST_FOR_EXCEPTION( !validEntry, Exceptions::InvalidParameterName ,"Error, the parameter {name=\""<name()<<"\"" "\nwas not found in the list of parameters during modification." "\n\nThe parameters and types are:\n" @@ -558,14 +558,14 @@ void ParameterList::reconcileParameterList(ParameterList & valid_pl, for (auto itr = valid_cur_node.begin(); itr != valid_cur_node.end(); ++itr){ const std::string &entry_name = itr->first; if (valid_cur_node.isSublist(entry_name)){ - const ParameterEntry &entry = itr->second; + const ParameterEntry &cur_entry = itr->second; ParameterList &valid_cur_node_sublist = valid_cur_node.sublist(entry_name); if (!valid_cur_node_sublist.disableRecursiveReconciliation_){ TEUCHOS_TEST_FOR_EXCEPTION_PURE_MSG( !cur_node.isSublist(entry_name), Exceptions::InvalidParameterName ,"Error, the parameter {name=\"" << entry_name <<"\"," - "type=\"" << entry.getAny(false).typeName() << "\"" - ",value=\"" << filterValueToString(entry) << "\"}" + "type=\"" << cur_entry.getAny(false).typeName() << "\"" + ",value=\"" << filterValueToString(cur_entry) << "\"}" "\nin the parameter (sub)list \"" <