Skip to content
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.

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

Group by discrete variables #405

Closed
wants to merge 2 commits into from
Closed

Group by discrete variables #405

wants to merge 2 commits into from

Conversation

TinyMarsh
Copy link
Contributor

This is by no means a PR with code suggestions. I just wanted to use the diff to facilitate discussion and to get help.

Context: #376. We need to add the ability to group by discrete variables, instead of just age and gender. So if I wanted to group by age and over_18, I could change the variable gender to over_18 and fetch the correct value from the Person object, i.e. entity.over_18.

Obviously this isn't going to work as is, for various reasons. Namely that series needs to be called with a gender, not a bool. And that we need to be able to group by other discrete variables in addition to age/gender, not instead of. But is this at least on the right track or am I barking up the wrong tree?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -276,41 +276,41 @@ void AnalysisModule::calculate_population_statistics(RuntimeContext &context,
auto current_time = static_cast<unsigned int>(context.time_now());
for (const auto &entity : context.population()) {
auto age = entity.age;
auto gender = entity.gender;
auto over_18 = entity.over_18;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: reference to non-static member function must be called; did you mean to call it with no arguments? [clang-diagnostic-error]

Suggested change
auto over_18 = entity.over_18;
auto over_18 = entity.over_18();

series(gender, "deaths").at(age)++;
auto expcted_life = definition_.life_expectancy().at(context.time_now(), gender);
series(gender, "yll").at(age) += std::max(expcted_life - age, 0.0f) * DALY_UNITS;
series(over_18, "deaths").at(age)++;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

                series(over_18, "deaths").at(age)++;
                ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

auto expcted_life = definition_.life_expectancy().at(context.time_now(), gender);
series(gender, "yll").at(age) += std::max(expcted_life - age, 0.0f) * DALY_UNITS;
series(over_18, "deaths").at(age)++;
auto expcted_life = definition_.life_expectancy().at(context.time_now(), over_18);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching member function for call to 'at' [clang-diagnostic-error]

                auto expcted_life = definition_.life_expectancy().at(context.time_now(), over_18);
                                                                  ^
Additional context

src/HealthGPS/gender_table.h:72: candidate function not viable: no known conversion from 'bool' to 'const core::Gender' for 2nd argument

    const TYPE &at(const ROW row, const core::Gender gender) const {
                ^

src/HealthGPS/gender_table.h:63: candidate function not viable: 'this' argument has type 'const GenderTable<int, float>', but method is not marked const

    TYPE &at(const ROW row, const core::Gender gender) {
          ^

series(gender, "yll").at(age) += std::max(expcted_life - age, 0.0f) * DALY_UNITS;
series(over_18, "deaths").at(age)++;
auto expcted_life = definition_.life_expectancy().at(context.time_now(), over_18);
series(over_18, "yll").at(age) += std::max(expcted_life - age, 0.0f) * DALY_UNITS;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

                series(over_18, "yll").at(age) += std::max(expcted_life - age, 0.0f) * DALY_UNITS;
                ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

}

if (entity.has_emigrated() && entity.time_of_migration() == current_time) {
series(gender, "migrations").at(age)++;
series(over_18, "migrations").at(age)++;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

                series(over_18, "migrations").at(age)++;
                ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^


for (const auto &factor : context.mapping().entries()) {
series(gender, factor.key().to_string()).at(age) +=
series(over_18, factor.key().to_string()).at(age) +=
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

            series(over_18, factor.key().to_string()).at(age) +=
            ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

entity.get_risk_factor_value(factor.key());
}

for (const auto &[disease_name, disease_state] : entity.diseases) {
if (disease_state.status == DiseaseStatus::active) {
series(gender, "prevalence_" + disease_name.to_string()).at(age)++;
series(over_18, "prevalence_" + disease_name.to_string()).at(age)++;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

                series(over_18, "prevalence_" + disease_name.to_string()).at(age)++;
                ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

if (disease_state.start_time == context.time_now()) {
series(gender, "incidence_" + disease_name.to_string()).at(age)++;
series(over_18, "incidence_" + disease_name.to_string()).at(age)++;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

                    series(over_18, "incidence_" + disease_name.to_string()).at(age)++;
                    ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

}
}
}

auto dw = calculate_disability_weight(entity);
series(gender, "disability_weight").at(age) += dw;
series(gender, "yld").at(age) += (dw * DALY_UNITS);
series(over_18, "disability_weight").at(age) += dw;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

        series(over_18, "disability_weight").at(age) += dw;
        ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

series(gender, "disability_weight").at(age) += dw;
series(gender, "yld").at(age) += (dw * DALY_UNITS);
series(over_18, "disability_weight").at(age) += dw;
series(over_18, "yld").at(age) += (dw * DALY_UNITS);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no matching function for call to object of type 'hgps::DataSeries' [clang-diagnostic-error]

        series(over_18, "yld").at(age) += (dw * DALY_UNITS);
        ^
Additional context

src/HealthGPS/data_series.h:29: candidate function not viable: no known conversion from 'bool' to 'core::Gender' for 1st argument

    std::vector<double> &operator()(core::Gender gender, const std::string &key);
                         ^

@alexdewar
Copy link
Contributor

Honestly, I don't know this part of the code at all so I have no idea! I'm happy to dig into it if needed though... @jamesturner246 how well do you know it?

@jamesturner246
Copy link
Contributor

Discussed this with @TinyMarsh yesterday. Will add some broken down issues.

@TinyMarsh TinyMarsh closed this Jun 5, 2024
@TinyMarsh TinyMarsh deleted the analysis_changes branch June 5, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants