search

optimamodel / optima-tb

Optima TB-UCL model
GNU Lesser General Public License v3.0
2 stars 0 forks source link

Extend impacts #102

Closed davidkedz closed 7 years ago

davidkedz commented 7 years ago

Programs can now target non-transition parameters, given the understanding that the parameters' values are directly overwritten by coverage, with no care for program attributes or impact-factor scaling, and no way of subdividing the overwriting value between target populations or impact groupings. In addition, a 'Minimum Value' and 'Maximum Value' column in the cascade book is now allowed for parameters, establishing limits for parameter values that are enforced once during the Model.build() phase (typically for user-supplied values) and once during the Model.updateValues() phase (generally for calculated functions and other updates).

critcortex commented 7 years ago

Similarly to #101 , would you be able to supply an example / test script @davidkedz ?

davidkedz commented 7 years ago

@critcortex: Sync up with tb-ucl-analyses/master and run test_art_special.py in the south_africa/Scripts folder, making sure you have selected the tb-ucl/extend-impacts branch. It should show outputs at the end relating to a program targeting non-transition parameter 'prop-art', as well as 'Minimum Value' and 'Maximum Value' constraints applied via the cascade book. As with #101, drop a note if the script is not working for you.

christiankuschel commented 7 years ago

@davidkedz The test script you mentioned 'throws' a warning when setting the min/max value in model.py for paramter 'D1_gD2': its 'vals' are all nan, so the comparison nan < min/max (or >) fails. Probably a simple 'skip if vals is not nan' line will do without compromising the functionality. It may be worthwhile to call 'np.seterr(all='raise')' (treats all warning as exceptions) for debugging purposes in the test scripts so that no warnings are lost when the logger is set to ignore warnings.

The rest of the changes look good and work.

davidkedz commented 7 years ago

@christiankuschel: Fair call. I was loose with the warnings as they had no impact on the model run, but there is indeed no point throwing them for normal operating procedures. (As a side note, error handling and logging across the codebase takes many awkwardly disparate forms, so it is on my bucket list to align them all at some point.)

So, to address this; rather than excepting all nan values, as it would be useful to have at least a warning at this stage if a user somehow manages to push nan-valued 'basic' parameters this far, I have allowed all parameters defined as functions of other parameters (i.e. those listed in settings.par_funcs) to remain unconstrained at this point. These parameters typically start off as np.nan in Model.build() as a user does not supply them values, thus throwing the warnings, but are calculated during Model.updateValues() and (as tested) are properly constrained during that iterative step anyway.

With this commit, the test case proceeds without warnings. As neither of us can see any other issue, I shall proceed with the merge.