InstituteforDiseaseModeling / covasim

COVID-19 Agent-based Simulator (Covasim): a model for exploring coronavirus dynamics and interventions
https://covasim.org
MIT License
250 stars 223 forks source link

ParseObj.update_pars() throwing KeyNotFoundError even when it is in pars?! #269

Closed rbelew closed 4 years ago

rbelew commented 4 years ago

Describe the bug

I am getting KeyNotFoundError: Key(s) exceptions, despite my pars containing these key/values?

To reproduce

I am trying to replicate the behavior described in the {panovska-griffiths20} and therefore use the following call sequence:

sim = create_sim(pars.best)

def create_sim(x):

# first I build pars = sc.objdict()
# including location and datafile attributes
# and (beta,pop_infected) parameters from x

sim = cv.Sim(pars=pars,datafile=datafile,location=location)

Note that this call to Sim() specifies both datafile and location parameters, AGAIN as separate parameters to Sim(), because otherwise it complained these were missing; Question#1: why?!

Here is the full error stack:

    Traceback (most recent call last):
    File "/Users/Shared/Relocated Items/Security/Data/virtualenv/covasim/src/cvsim.py", line 325, in <module>
    sim = create_sim(pars.best)
    File "/Users/Shared/Relocated Items/Security/Data/virtualenv/covasim/src/cvsim.py", line 129, in create_sim
    sim = cv.Sim(pars=pars,datafile=CurrCtnyParams['datafile'],location=CurrCtnyParams['location'])
    File "/Users/Shared/Relocated Items/Security/Data/virtualenv/covasim/lib/python3.7/site-packages/covasim/sim.py", line 68, in __init__
    self.update_pars(pars, **kwargs)   # Update the parameters, if provided
    File "/Users/Shared/Relocated Items/Security/Data/virtualenv/covasim/lib/python3.7/site-packages/covasim/sim.py", line 85, in update_pars
    super().update_pars(pars=pars, create=create) # Call update_pars() for ParsObj
    File "/Users/Shared/Relocated Items/Security/Data/virtualenv/covasim/lib/python3.7/site-packages/covasim/base.py", line 70, in update_pars
    raise sc.KeyNotFoundError(errormsg)
    sciris.sc_utils.KeyNotFoundError: Key(s) ['storage', 'tot_pop'] not found; available keys are ['pop_size', 'pop_infected', 'pop_type', 'location', 'start_day', 'end_day', 'n_days', 'rand_seed', 'verbose', 'pop_scale', 'rescale', 'rescale_threshold', 'rescale_factor', 'beta', 'contacts', 'dynam_layer', 'beta_layer', 'n_imports', 'beta_dist', 'viral_dist', 'asymp_factor', 'iso_factor', 'quar_factor', 'quar_period', 'dur', 'rel_symp_prob', 'rel_severe_prob', 'rel_crit_prob', 'rel_death_prob', 'prog_by_age', 'prognoses', 'interventions', 'analyzers', 'timelimit', 'stopping_func', 'n_beds_hosp', 'n_beds_icu', 'no_hosp_factor', 'no_icu_factor']

I've added debug output to Sim.update_pars() just before it calls super().update_pars():

print('sim_pars',sorted(list(pars.keys())))
super().update_pars(pars=pars, create=create) # Call update_pars() for ParsObj

I've also added debug output to ParseObj.update_pars()

        if not create:
            available_keys = list(self.pars.keys())
            print('base_pars',sorted(pars.keys()))
            print('base_available',sorted(available_keys))

This results in the Sim.update_pars() output to be produced twice, the first time with 39 keys, and the second time with only 19, when create_sim is called again by the objective() function required for optuna.

sim_pars1 = ['analyzers', 'asymp_factor', 'beta', 'beta_dist', 'beta_layer', 'contacts', 'dur', 'dynam_layer', 'end_day', 'interventions', 'iso_factor', 'location', 'n_beds_hosp', 'n_beds_icu', 'n_days', 'n_imports', 'no_hosp_factor', 'no_icu_factor', 'pop_infected', 'pop_scale', 'pop_size', 'pop_type', 'prog_by_age', 'prognoses', 'quar_factor', 'quar_period', 'rand_seed', 'rel_crit_prob', 'rel_death_prob', 'rel_severe_prob', 'rel_symp_prob', 'rescale', 'rescale_factor', 'rescale_threshold', 'start_day', 'stopping_func', 'timelimit', 'verbose', 'viral_dist']

sim_pars2 = ['asymp_factor', 'beta', 'beta_layer', 'contacts', 'dynam_layer', 'end_day', 'interventions', 'iso_factor', 'location', 'pop_infected', 'pop_scale', 'pop_size', 'pop_type', 'quar_factor', 'rescale', 'start_day', 'storage', 'tot_pop', 'verbose']

The output from the ParseObj.update_pars() output show

    base_pars = ['asymp_factor', 'beta', 'beta_layer', 'contacts', 'dynam_layer', 'end_day', 'interventions', 'iso_factor', 'location', 'pop_infected', 'pop_scale', 'pop_size', 'pop_type', 'quar_factor', 'rescale', 'start_day', 'storage', 'tot_pop', 'verbose']

base_available = ['analyzers', 'asymp_factor', 'beta', 'beta_dist', 'beta_layer', 'contacts', 'dur', 'dynam_layer', 'end_day', 'interventions', 'iso_factor', 'location', 'n_beds_hosp', 'n_beds_icu', 'n_days', 'n_imports', 'no_hosp_factor', 'no_icu_factor', 'pop_infected', 'pop_scale', 'pop_size', 'pop_type', 'prog_by_age', 'prognoses', 'quar_factor', 'quar_period', 'rand_seed', 'rel_crit_prob', 'rel_death_prob', 'rel_severe_prob', 'rel_symp_prob', 'rescale', 'rescale_factor', 'rescale_threshold', 'start_day', 'stopping_func', 'timelimit', 'verbose', 'viral_dist']

Question#2: I'm not sure of the intent of distinguishing between the passed pars value and self.pars, leading to the KeyNotFoundError?

For now, I have just removed that test, and everything seems to work.

Expected behavior

The call to Sim() should make use of the pars dictionary provided.

Platform (please complete the following information):

cliffckerr commented 4 years ago

Hi @rbelew , thanks for raising this. The original error seems to be caused by:

sciris.sc_utils.KeyNotFoundError: Key(s) ['storage', 'tot_pop'] not found

Are the keys 'storage' and 'tot_pop' present in the parameters object you are supplying? The only parameters that should be entered in the pars object are those present by default:

import covasim as cv
sim = cv.Sim()
sim.pars.keys()
>>> dict_keys(['pop_size', 'pop_infected', 'pop_type', 'location', 'start_day', 'end_day', 'n_days', 'rand_seed', 'verbose', 'pop_scale', 'rescale', 'rescale_threshold', 'rescale_factor', 'beta', 'contacts', 'dynam_layer', 'beta_layer', 'n_imports', 'beta_dist', 'viral_dist', 'asymp_factor', 'iso_factor', 'quar_factor', 'quar_period', 'dur', 'rel_symp_prob', 'rel_severe_prob', 'rel_crit_prob', 'rel_death_prob', 'prog_by_age', 'prognoses', 'interventions', 'analyzers', 'timelimit', 'stopping_func', 'n_beds_hosp', 'n_beds_icu', 'no_hosp_factor', 'no_icu_factor'])

The data file and location are properties of the simulation, not model parameters, so they should only be supplied as arguments to Sim() and should not be present in the parameters object. Storage only pertains to Optuna, and should also not be part of the pars object. The pars object passed to Sim() is indeed used to population sim.pars, and the error occurs when you pass an invalid or unrecognized value to it (here, 'storage' and 'tot_pop').

Hope that answers your question but if not happy to provide more information.

rbelew commented 4 years ago

Got it! yes, i was using pars for both simulation and model parameters. thanks for your quick response.

and since this is my first but i am sure not last interaction with you: THANKS, THANKS for your group's work on covasim!! between the code base and the {panovska-griffiths20} publication, it is a game changer!

cliffckerr commented 4 years ago

You're most welcome @rbelew! Happy to be of help and let us know if you have more questions.