Skip to content

Added small utility to verify vector access #87

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Added small utility to verify vector access #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 10 additions & 9 deletions src/IO.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,13 @@ ObservationsForInference ReadInferenceObservations(const std::string& configDir,
int nHealthBoards = validationParams.nHealthBoards;
int nCasesDays = validationParams.nCasesDays;

unsigned int cases_rows = observations.cases.size();
unsigned int cases_rows = Utilities::checkAndGetSize(observations.cases, "observations.cases");
Copy link
Contributor

@peter-t-fox peter-t-fox Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused by this. It seems that checkAndGetSize is just doing a subset of what the ImportConsistencyCheck function is doing a few lines down? (The former only checks that there are a non-zero number of rows in observations.cases; the latter checks that there are exactly the correct number of rows and columns).

Copy link
Contributor Author

@kzscisoft kzscisoft Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current ordering will cause a memory issue if the number of rows is zero:

unsigned int cases_rows = observations.cases.size();
unsigned int cases_cols = observations.cases[0].size();

I agree my method might not be needed but if you plan to drop it then we need to reorder so that the rows are checked before the columns are retrieved, i.e.:

unsigned int cases_rows = observations.cases.size();
ImportConsistencyCheck(scot_data_file, cases_rows, (nHealthBoards + 1), "rows");
unsigned int cases_cols = observations.cases[0].size();
ImportConsistencyCheck(scot_deaths_file, deaths_cols, nCasesDays, "columns");

instead.

unsigned int cases_cols = observations.cases[0].size();

ImportConsistencyCheck(scot_data_file, cases_rows, (nHealthBoards + 1), "rows");
ImportConsistencyCheck(scot_data_file, cases_cols, nCasesDays, "columns");

unsigned int deaths_rows = observations.deaths.size();
unsigned int deaths_rows = Utilities::checkAndGetSize(observations.deaths, "observations.deaths");
unsigned int deaths_cols = observations.deaths[0].size();

ImportConsistencyCheck(scot_deaths_file, deaths_rows, (nHealthBoards + 1), "rows");
Expand Down Expand Up @@ -267,7 +267,8 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << scot_data_file << std::endl;
observations.cases = Utilities::read_csv<int>(scot_data_file, ',');

unsigned int cases_rows = observations.cases.size();
unsigned int cases_rows = Utilities::checkAndGetSize(observations.cases, "observations.cases");

unsigned int cases_cols = observations.cases[0].size();

ImportConsistencyCheck(scot_data_file, cases_rows, (nHealthBoards + 1), "rows");
Expand All @@ -280,7 +281,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << scot_ages_file << std::endl;
observations.age_pop = Utilities::read_csv<double>(scot_ages_file, ',');

unsigned int age_pop_rows = observations.age_pop.size();
unsigned int age_pop_rows = Utilities::checkAndGetSize(observations.age_pop, "observations.age_pop");
unsigned int age_pop_cols = observations.age_pop[0].size();

ImportConsistencyCheck(scot_ages_file, age_pop_rows, nHealthBoards, "rows");
Expand All @@ -291,7 +292,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << waifw_norm_file << std::endl;
observations.waifw_norm = Utilities::read_csv<double>(waifw_norm_file, ',');

unsigned int waifw_norm_rows = observations.waifw_norm.size();
unsigned int waifw_norm_rows = Utilities::checkAndGetSize(observations.waifw_norm, "observations.waifw_norm");
unsigned int waifw_norm_cols = observations.waifw_norm[0].size();

ImportConsistencyCheck(waifw_norm_file, waifw_norm_rows, nAgeGroups, "rows");
Expand All @@ -301,7 +302,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << waifw_home_file << std::endl;
observations.waifw_home = Utilities::read_csv<double>(waifw_home_file, ',');

unsigned int waifw_home_rows = observations.waifw_home.size();
unsigned int waifw_home_rows = Utilities::checkAndGetSize(observations.waifw_home, "observations.waifw_home");
unsigned int waifw_home_cols = observations.waifw_home[0].size();

ImportConsistencyCheck(waifw_home_file, waifw_home_rows, nAgeGroups, "rows");
Expand All @@ -311,7 +312,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << waifw_sdist_file << std::endl;
observations.waifw_sdist = Utilities::read_csv<double>(waifw_sdist_file, ',');

unsigned int waifw_sdist_rows = observations.waifw_sdist.size();
unsigned int waifw_sdist_rows = Utilities::checkAndGetSize(observations.waifw_sdist, "observations.waifw_sdist");
unsigned int waifw_sdist_cols = observations.waifw_sdist[0].size();

ImportConsistencyCheck(waifw_sdist_file, waifw_sdist_rows, nAgeGroups, "rows");
Expand All @@ -325,7 +326,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << cfr_byage_file << std::endl;
observations.cfr_byage = Utilities::read_csv<double>(cfr_byage_file, ',');

unsigned int cfr_rows = observations.cfr_byage.size();
unsigned int cfr_rows = Utilities::checkAndGetSize(observations.cfr_byage, "observations.cfr_byage");
unsigned int cfr_cols = observations.cfr_byage[0].size();

ImportConsistencyCheck(cfr_byage_file, cfr_rows, nAgeGroups, "rows");
Expand All @@ -338,7 +339,7 @@ ObservationsForModels ReadModelObservations(const std::string& configDir, Utilit
(*log) << "\t- " << scot_frail_file << std::endl;
observations.pf_pop = Utilities::read_csv<double>(scot_frail_file, ',');

unsigned int pf_pop_rows = observations.pf_pop.size();
unsigned int pf_pop_rows = Utilities::checkAndGetSize(observations.pf_pop, "observations.pf_pop");
unsigned int pf_pop_cols = observations.pf_pop[0].size();

ImportConsistencyCheck(scot_frail_file, pf_pop_rows, nHealthBoards, "rows");
Expand Down
2 changes: 2 additions & 0 deletions src/IrishModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ IrishModel::IrishModel(const CommonModelInputParameters& commonParameters,
fixedParameters_ = BuildFixedParameters(
observations.waifw_norm.size(), commonParameters.paramlist
);

Utilities::checkAndGetSize(observations.pf_pop, "observations.pf_pop");
Copy link
Contributor

@peter-t-fox peter-t-fox Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notwithstanding my comment above, I'd suggest that once you have checked that the data are valid once in the IO code, there's no reason to check again in the model code.


ageGroupData_ = AgeGroupData{
observations.waifw_norm,
Expand Down
4 changes: 4 additions & 0 deletions src/ModelCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ std::vector<int> ComputeAgeNums(int shb_id, int Npop, int N_hcw, const Observati

// define age structure of the shb of interest. the -1 is to account for difference in number of
// rows between two datasets (age_pop does not have a row with column name)
Utilities::checkAndGetSize(obs.age_pop);
const auto& agedist = obs.age_pop[shb_id - 1];

for (const auto& var : agedist) {
Expand All @@ -41,7 +42,10 @@ int GetPopulationOfRegion(const ObservationsForModels& obs, int region_id)
int ComputeNumberOfHCWInRegion(int regionalPopulation, int totalHCW, const ObservationsForModels& obs)
{
int scotlandPopulation = 0;
Utilities::checkAndGetSize(obs.cases, "obs.cases");

for (unsigned int region = 0; region < obs.cases.size() - 1; ++region) {
Utilities::checkAndGetSize(obs.cases[region], "obs.cases["+std::to_string(region)+"]");
scotlandPopulation += obs.cases[region][0];
}
double regionalProportion = static_cast<double>(regionalPopulation) / scotlandPopulation;
Expand Down
1 change: 1 addition & 0 deletions src/ModelCommon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "ModelTypes.h"
#include "Utilities.h"
#include "Random.h"
#include <vector>
#include <memory>
Expand Down
4 changes: 3 additions & 1 deletion src/OriginalModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ OriginalModel::OriginalModel(const CommonModelInputParameters& commonParameters,
fixedParameters_ = BuildFixedParameters(
observations.waifw_norm.size(), commonParameters.paramlist
);


Utilities::checkAndGetSize(observations.pf_pop, "observations.pf_pop");

ageGroupData_ = AgeGroupData{
observations.waifw_norm,
observations.waifw_home,
Expand Down
2 changes: 2 additions & 0 deletions src/TempModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ TempModel::TempModel(const CommonModelInputParameters& commonParameters,
fixedParameters_ = BuildFixedParameters(
observations.waifw_norm.size(), commonParameters.paramlist
);

Utilities::checkAndGetSize(observations.pf_pop, "observations.pf_pop");

ageGroupData_ = AgeGroupData{
observations.waifw_norm,
Expand Down
21 changes: 21 additions & 0 deletions src/Utilities.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,27 @@ std::vector<T> AccumulateEveryN(const std::vector<T>& data, unsigned int n)
return _temp;
}

/**
* @brief Verifying container access
*
* Determines if container is non-zero before attempting access
* if successful the size of the container is returned.
*
* @param container container object, e.g. STL vector
*
* @return size of the container
*/
template<typename T>
unsigned int checkAndGetSize(const T& container, std::string name="container")
{
if(container.size() < 1)
{
throw std::out_of_range("Cannot access elements of '"+name+"' object which is size zero.");
}

return container.size();
}

/*! @brief Logging Stream Class
@details Class to send output to both cout and output log file
@date last modified 2020-05-18
Expand Down
1 change: 1 addition & 0 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ int main(int argc, char** argv)

// Import model observational data
std::string modelConfigDir(std::string(ROOT_DIR) + "/data");

ObservationsForModels modelObservations = IO::ReadModelObservations(modelConfigDir, logger);

// Log the disease seed settings
Expand Down