Is context.get_person_id(usize_id) necessary?

[ChiragKumar9]

The challenges described in #60 arises because context.get_person_id(usize_id) returns PersonId{ id: usize_id} even if that person doesn't technically exist. Given a valid PersonId, we can query properties for that person in from PeopleData. Right now, we need this functionality for the following kinds of patterns:

1. Count the number of people who have a certain property value

// count the number of people who are infected
// could also write this with an iter and collect
// but to be explicit i will not use that syntax here
let mut counter = 0;
for usize_id in 0..context.get_current_population() {
    if matches!(context.get_person_property(context.get_person_id(usize_id), InfectionStatusType),
                       InfectionStatus::I) {
        counter += 1;
}

2. Identify a person who can be infected by randomly drawing Ids.

fn attempt_infection(context: &mut Context) {
    let population_size: usize = context.get_population();
    let person_to_infect: usize = context.sample_range(TransmissionRng, 0..population_size);

    let person_status: InfectionStatus = context.get_person_property(context.get_person_id(person_to_infect),
                                                                                                            InfectionStatusType);

    if matches!(person_status, InfectionStatus::S) {
        context.set_person_property(context.get_person_id(person_to_infect), InfectionStatusType,
            InfectionStatus::I);
    }
}

I believe both functionalities can be addressed by alternative syntax, meaning we can get rid of context.get_person_id(...).

In essence, 1) is asking us to get the number of people who have a given person property value. This is just the size of a partition defined by the given person property values. If we want the entire set of people in the simulation, we could define some partition that is the entire set of alive people, and take it from there. I believe this may also negate the need for context.get_current_population(). Here's what some code could look like:

define_partition!(Infecteds, InfectionStatusType == InfectionStatus::I);
let counter = context.get_partition(Infecteds).len();

2. is asking us for something quite similar: given all people in the simulation, return a random one. First of all, Add helper for getting a random person #55 indicates that such a helper is in the works. But, we could use the same partition idea:

define_partition!(AllPeopleAlive, Alive);
let person_to_infect = context.sample_population(InfectionRng, context.get_partition(AllPeopleAlive));

This may be overkill! And, this all assumes we don't have a way to remove people (i.e., context.remove_person) but rather just keep removed people in the simulation but with some property flag that marks them as not in the simulation (i.e., dead) and from there we can create partitions that filter such people out.

[ekr-cfa]

Thanks for catching this. I generally agree with your argument here.

1. The cardinality problem you indicate is indeed soluble with queries/indexes: https:/CDCgov/ixa/blob/k88hudson_query/src/people.rs#L865
2. I agree that we want context.sample_population() though less certain about the precise syntax you propose.

In the short term, I would have get_person_id() fail when the input ID is invalid. It can panic as we probably will just want to remove the API as you suggest, though ordinarily I would want to return Result<PersonId>