From b10c22dddefee79f657a66ab2c58f38baf1d94d0 Mon Sep 17 00:00:00 2001 From: Paul Nathan Stickney Date: Tue, 18 Aug 2020 12:12:05 -0700 Subject: [PATCH] #990 lb reader- remove theConfig dep and avoid ParallelTestHarness - The spec API has changed slightly to increase encapsulation. The 'openSpec(filename)' call now replaces disparate calls to read/check the specification and avoid any internal assumptions about which file is read. --- .../vrt/collection/balance/baselb/baselb.cc | 2 +- .../balance/lb_invoke/lb_manager.cc | 5 +- src/vt/vrt/collection/balance/read_lb.cc | 52 +++++++++---------- src/vt/vrt/collection/balance/read_lb.h | 16 ++++-- tests/unit/lb/test_lb_reader.nompi.cc | 11 ++-- 5 files changed, 46 insertions(+), 40 deletions(-) diff --git a/src/vt/vrt/collection/balance/baselb/baselb.cc b/src/vt/vrt/collection/balance/baselb/baselb.cc index 54fe2b8941..152777dda7 100644 --- a/src/vt/vrt/collection/balance/baselb/baselb.cc +++ b/src/vt/vrt/collection/balance/baselb/baselb.cc @@ -121,7 +121,7 @@ void BaseLB::importProcessorData( void BaseLB::getArgs(PhaseType phase) { using namespace balance; - bool has_spec = ReadLBSpec::hasSpec(); + bool has_spec = ReadLBSpec::openSpec(theConfig()->vt_lb_file_name); if (has_spec) { auto spec = ReadLBSpec::entry(phase); if (spec) { diff --git a/src/vt/vrt/collection/balance/lb_invoke/lb_manager.cc b/src/vt/vrt/collection/balance/lb_invoke/lb_manager.cc index f802b681db..24da652b47 100644 --- a/src/vt/vrt/collection/balance/lb_invoke/lb_manager.cc +++ b/src/vt/vrt/collection/balance/lb_invoke/lb_manager.cc @@ -106,8 +106,9 @@ LBType LBManager::decideLBToRun(PhaseType phase, bool try_file) { return LBType::NoLB; } - if (theConfig()->vt_lb_file_name != "" and try_file) { - bool const has_spec = ReadLBSpec::hasSpec(); + auto& spec_file = theConfig()->vt_lb_file_name; + if (spec_file != "" and try_file) { + bool const has_spec = ReadLBSpec::openSpec(spec_file); if (has_spec) { the_lb = ReadLBSpec::getLB(phase); } diff --git a/src/vt/vrt/collection/balance/read_lb.cc b/src/vt/vrt/collection/balance/read_lb.cc index bc0917e414..cdb1807e2c 100644 --- a/src/vt/vrt/collection/balance/read_lb.cc +++ b/src/vt/vrt/collection/balance/read_lb.cc @@ -57,41 +57,44 @@ namespace vt { namespace vrt { namespace collection { namespace balance { -/*static*/ std::string ReadLBSpec::filename = {}; +/*static*/ std::string ReadLBSpec::open_filename_ = {}; /*static*/ typename ReadLBSpec::SpecMapType ReadLBSpec::spec_mod_ = {}; /*static*/ typename ReadLBSpec::SpecMapType ReadLBSpec::spec_exact_ = {}; /*static*/ std::vector ReadLBSpec::spec_prec_ = {}; /*static*/ bool ReadLBSpec::read_complete_ = false; -/*static*/ bool ReadLBSpec::hasSpec() { - if (read_complete_) { +/*static*/ bool ReadLBSpec::openSpec(std::string const& filename) { + // Ignore attempt to open same spec. + if (not open_filename_.empty() and open_filename_ == filename) { return true; } - auto const file_name = theConfig()->vt_lb_file_name; - if (file_name == "") { + // No-op if no file specified. Can't be used to clear. + if (filename.empty()) { return false; } - bool good = openFile(file_name); - if (not good) { - auto str = - fmt::format("--vt_lb_file_name={} is not a valid file", file_name); + vtAssert( + open_filename_.empty(), + "Spec already opened. Use clear first to load again." + ); + + // Ensure file can be opened. + std::ifstream file(filename); + if (not file.good()) { + auto str = fmt::format("Unable to open spec file: {}", filename); vtAbort(str); } - return good; -} -/*static*/ bool ReadLBSpec::openFile(std::string const name) { - std::ifstream file(name); - filename = name; - return file.good(); + // Remember loaded file - multiple calls to same file are idempotent. + open_filename_ = filename; + + readFile(filename); + + return true; } /*static*/ LBType ReadLBSpec::getLB(SpecIndex const& idx) { - if (not read_complete_) { - readFile(); - } auto const lb = entry(idx); if (lb) { return lb->getLB(); @@ -133,11 +136,7 @@ int eatWhitespace(std::ifstream& file) { return file.eof() ? 0 : file.peek(); } -/*static*/ void ReadLBSpec::readFile() { - if (read_complete_) { - return; - } - +/*static*/ void ReadLBSpec::readFile(std::string const& filename) { std::ifstream file(filename); vtAssert(file.good(), "must be valid"); @@ -245,7 +244,7 @@ int eatWhitespace(std::ifstream& file) { /*static*/ void ReadLBSpec::clear() { read_complete_ = false; - filename = ""; + open_filename_ = ""; spec_mod_.clear(); spec_exact_.clear(); spec_prec_.clear(); @@ -332,8 +331,9 @@ auto excluded_str = [](SpecIndex idx) -> std::string { /*static*/ std::string ReadLBSpec::toString() { std::stringstream ss; - ReadLBSpec::openFile(theConfig()->vt_lb_file_name); - ReadLBSpec::readFile(); + if (open_filename_.empty()) { + return ""; + } if (not ReadLBSpec::getExactEntries().empty()) { ss << fmt::format("{}\tExact specification lines:\n", vt::debug::vtPre()); diff --git a/src/vt/vrt/collection/balance/read_lb.h b/src/vt/vrt/collection/balance/read_lb.h index d7c2602395..b552faf564 100644 --- a/src/vt/vrt/collection/balance/read_lb.h +++ b/src/vt/vrt/collection/balance/read_lb.h @@ -181,10 +181,14 @@ struct ReadLBSpec { using SpecMapType = std::map; using ParamMapType = std::map; - static bool openFile(std::string const name = ""); - static void readFile(); + /** + * \brief Prepare the spec for reading, if it exists. + * + * This will also ensure the spec is loaded. + * This method MUST be called before the other methods. + */ + static bool openSpec(std::string const& filename); - static bool hasSpec(); static SpecIndex numEntries() { return spec_mod_.size() + spec_exact_.size(); } static SpecEntry* entry(SpecIndex const& idx); static LBType getLB(SpecIndex const& idx); @@ -192,12 +196,14 @@ struct ReadLBSpec { static SpecMapType getExactEntries() {return spec_exact_; }; static ParamMapType parseParams(std::vector params); static SpecEntry makeSpecFromParams(std::string params); - static void clear(); static std::string toString(); + static void clear(); private: + static void readFile(std::string const& filename); + static bool read_complete_; - static std::string filename; + static std::string open_filename_; static SpecMapType spec_mod_; static SpecMapType spec_exact_; static std::vector spec_prec_; diff --git a/tests/unit/lb/test_lb_reader.nompi.cc b/tests/unit/lb/test_lb_reader.nompi.cc index 66cd4b1050..175b2ce5e1 100644 --- a/tests/unit/lb/test_lb_reader.nompi.cc +++ b/tests/unit/lb/test_lb_reader.nompi.cc @@ -45,11 +45,11 @@ #include #include -#include "test_parallel_harness.h" +#include "test_harness.h" namespace vt { namespace tests { namespace unit { -using TestLBReader = TestParallelHarness; +using TestLBReader = TestHarness; TEST_F(TestLBReader, test_lb_read_1) { @@ -64,9 +64,9 @@ TEST_F(TestLBReader, test_lb_read_1) { using Spec = vt::vrt::collection::balance::ReadLBSpec; using SpecIdx = vt::vrt::collection::balance::SpecIndex; using SpecLBType = vt::vrt::collection::balance::LBType; + Spec::clear(); - EXPECT_EQ(Spec::openFile(file_name), true); - Spec::readFile(); + Spec::openSpec(file_name); EXPECT_EQ(Spec::numEntries(), 3); EXPECT_EQ(Spec::getExactEntries().size(), 2); @@ -121,8 +121,7 @@ TEST_F(TestLBReader, test_lb_read_2) { using SpecIdx = vt::vrt::collection::balance::SpecIndex; using SpecLBType = vt::vrt::collection::balance::LBType; Spec::clear(); - Spec::openFile(file_name); - Spec::readFile(); + Spec::openSpec(file_name); EXPECT_EQ(Spec::numEntries(), 5); for (SpecIdx i = 0; i < 121; i++) {