Skip to content

Commit

Permalink
#990 lb reader- remove theConfig dep and avoid ParallelTestHarness
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
pnstickne committed Aug 21, 2020
1 parent 575727c commit b10c22d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/vt/vrt/collection/balance/baselb/baselb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/vt/vrt/collection/balance/lb_invoke/lb_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
52 changes: 26 additions & 26 deletions src/vt/vrt/collection/balance/read_lb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<SpecIndex> 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();
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down
16 changes: 11 additions & 5 deletions src/vt/vrt/collection/balance/read_lb.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,23 +181,29 @@ struct ReadLBSpec {
using SpecMapType = std::map<SpecIndex,SpecEntry>;
using ParamMapType = std::map<std::string, std::string>;

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);
static SpecMapType getModEntries() { return spec_mod_; };
static SpecMapType getExactEntries() {return spec_exact_; };
static ParamMapType parseParams(std::vector<std::string> 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<SpecIndex> spec_prec_;
Expand Down
11 changes: 5 additions & 6 deletions tests/unit/lb/test_lb_reader.nompi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@
#include <vt/transport.h>
#include <vt/vrt/collection/balance/read_lb.h>

#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) {

Expand All @@ -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);
Expand Down Expand Up @@ -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++) {
Expand Down

0 comments on commit b10c22d

Please sign in to comment.