Implement stats calculations for new calculated_stats_ container

[TinyMarsh]

This PR adds much of the functionality to calculate the stats specified in channels_. A lot of the logic for this is copied from the existing code, but modified to use the calculated_stats_ vector in place of the existing series object. As a reminder, this is necessary because the calculated stats are being stored in a flat vector (calculated_stats_) which represents a multidimensional array of factors that the user wants to group by, as opposed to the series object. A result of this is the introduction of the calculate_index method, and the get_channel_index method - both necessary to find the correct index of the factor bin, and "channel" stat in the calculated_stats_ vector, respectively.

[github-actions]

clang-tidy made some suggestions

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: function 'calculate_population_statistics' has cognitive complexity of 29 (threshold 25) [readability-function-cognitive-complexity]

void AnalysisModule::calculate_population_statistics(RuntimeContext &context) {
                     ^

Additional context

src/HealthGPS/analysis_module.cpp:324: +1, including nesting penalty of 0, nesting level increased to 1

    for (const auto &person : context.population()) {
    ^

src/HealthGPS/analysis_module.cpp:329: +2, including nesting penalty of 1, nesting level increased to 2

        if (!person.is_active()) {
        ^

src/HealthGPS/analysis_module.cpp:330: +3, including nesting penalty of 2, nesting level increased to 3

            if (!person.is_alive() && person.time_of_death() == current_time) {
            ^

src/HealthGPS/analysis_module.cpp:330: +1

            if (!person.is_alive() && person.time_of_death() == current_time) {
                                   ^

src/HealthGPS/analysis_module.cpp:339: +3, including nesting penalty of 2, nesting level increased to 3

            if (person.has_emigrated() && person.time_of_migration() == current_time) {
            ^

src/HealthGPS/analysis_module.cpp:339: +1

            if (person.has_emigrated() && person.time_of_migration() == current_time) {
                                       ^

src/HealthGPS/analysis_module.cpp:348: +2, including nesting penalty of 1, nesting level increased to 2

        for (const auto &factor : context.mapping().entries()) {
        ^

src/HealthGPS/analysis_module.cpp:354: +2, including nesting penalty of 1, nesting level increased to 2

        for (const auto &[disease_name, disease_state] : person.diseases) {
        ^

src/HealthGPS/analysis_module.cpp:355: +3, including nesting penalty of 2, nesting level increased to 3

            if (disease_state.status == DiseaseStatus::active) {
            ^

src/HealthGPS/analysis_module.cpp:358: +4, including nesting penalty of 3, nesting level increased to 4

                if (disease_state.start_time == context.time_now()) {
                ^

src/HealthGPS/analysis_module.cpp:374: +1, including nesting penalty of 0, nesting level increased to 1

    for (size_t i = 0; i < calculated_stats_.size(); i += channels_.size()) {
    ^

src/HealthGPS/analysis_module.cpp:381: +2, including nesting penalty of 1, nesting level increased to 2

        for (const auto &factor : context.mapping().entries()) {
        ^

src/HealthGPS/analysis_module.cpp:387: +2, including nesting penalty of 1, nesting level increased to 2

        for (const auto &disease : context.diseases()) {
        ^

src/HealthGPS/analysis_module.cpp:399: +2, including nesting penalty of 1, nesting level increased to 2

        for (const auto &column : {"mean_yll", "mean_yld", "mean_daly"}) {
        ^

[alexdewar]

I think clang-tidy is right that this function is a bit over-complicated... Could you maybe split it up?

[alexdewar]

Nearly there, I think. I do think it would be worth adding a test for the calculate_index function though and I've made a few other small suggestions.

The Windows CI runner is failing because of some type narrowing warning again 😞

[alexdewar]

I think clang-tidy is right that this function is a bit over-complicated... Could you maybe split it up?

[alexdewar]

Could this be replaced with just channel_index_.at(channel)?

[alexdewar]

I think you can get away without a break after a throw:

Suggested change

break;

[alexdewar]

What's the difference between over_weight and above_weight?

[alexdewar]

Could you maybe add a unit test for this function to HealthGPS.Tests? I've been pretty bad at writing tests for HealthGPS myself, but this one should be easy enough to write a test for and then we could be a bit more confident it's doing the right thing.

[alexdewar]

I might not be completely understanding what these data structures do... But am I right in thinking channel_index_ contains indexes into channels_? If so, could you just make channels_ a std::map<std::string, std::string> (I'm guessing it needs to be ordered...) and drop channels_index_?