Fix YLL/Y:D/DALY and death

[jamesturner246]

Fixes #442.

The loops for mean and std have been tidied up, and are now 'opt-in', so no unwanted surprises in other columns that don't opt in.

YLL/YLD/DALY are now correctly computed by dividing by active PLUS deceased population.

Everything that should be an integer, such as deaths, migrations, weight classifications, is now an integer. Update your scripts accordingly.

[jamesturner246]

Don't they do CI on these Ubuntu builder images? 😕

 Preparing to unpack .../00-firefox_1%3a1snap1-0ubuntu2_amd64.deb ...
=> Installing the firefox snap
==> Checking connectivity with the snap store
===> Unable to contact the store, trying every minute for the next 30 minutes
==> Installing the firefox snap
error: cannot install "firefox": cannot query the store for updates: got
       unexpected HTTP status code 503 via POST to
       "https://api.snapcraft.io/v2/snaps/refresh"
dpkg: error processing archive /tmp/apt-dpkg-install-V2b2KG/00-firefox_1%3a1snap1-0ubuntu2_amd64.deb (--unpack):
 new firefox package pre-installation script subprocess returned error exit status 1
Please restart all running instances of firefox, or you will experience problems.

[jzhu20]

I don't really understand the codes for calculating standard deviation.
Lines 452-456 and 458-461: Do you substract individual value from group mean, then take squared to generate sum? I cannot locate this step. I get sqrt(sum/count) or sqrt(sum/(count+deaths)). @jamesturner246

[jamesturner246]

Hi @jzhu20, this was done in a prior PR -- see lines 402-434.

[jamesturner246]

Any thoughts @alexdewar?

[alexdewar]

Sorry for the slow reply! I'll take a look at this today

[alexdewar]

I've eyeballed the code and it looks like it's doing the right thing. I've made some comments, none of which are actually about this PR 🙃

That said, the calculate_population_statistics() method is a bit overcomplicated (we have to suppress a clang-tidy warning about this), so trying to keep track of everything that's going on made my head hurt. We should probably factor out some of the logic into separate functions at some point. Maybe we need a new issue for this?

[alexdewar]

Not relevant for this PR, but I feel like a has_died() method would be clearer than an is_alive() one.

[jamesturner246]

Then you'd have is_alive and has_died = !is_alive functions?

Unless you mean like:

bool Person::has_just_died(unsigned int current_time) {
    return !is_alive() && time_of_death() == current_time;
}

[alexdewar]

Don't we need disability_weight anymore?

[jamesturner246]

One can just divide YLD by 100000 to get it if they still want. I understand YLD is the actually useful one.

[alexdewar]

I think it would make more sense to return a new DataSeries object rather than modify an input argument.

But that's not something we need to do now.

[jamesturner246]

That said, the calculate_population_statistics() method is a bit overcomplicated (we have to suppress a clang-tidy warning about this), so trying to keep track of everything that's going on made my head hurt. We should probably factor out some of the logic into separate functions at some point. Maybe we need a new issue for this?

Agree in principle, but I'm hesitant to invest any more time in this whilst knowing it will eventually be replaced by @TinyMarsh's work. Let's wait for the result of that before making any decisions.