fix for running autocalibrate functionality

optimamodel / optima-tb

Optima TB-UCL model

GNU Lesser General Public License v3.0

2 stars 0 forks source link

fix for running autocalibrate functionality #87

Closed hussainazfar closed 7 years ago

hussainazfar commented 7 years ago

the asd algorithm was modified at some point when programs were introduced into the model
this update intends to make sure that the asd for autocalibrate functionality remains in working condition
will need to collapse the asd algorithm into one

davidkedz commented 7 years ago

Interesting issue. ASD was indeed upgraded to the HIV version upon the advice of @cliffckerr when optimisations were being set up. I don't exactly recall, but I think the signature would not have changed by much. Therefore, the ideal fix is to have autocalibration calls to ASD be appropriately modified, rather than having two different ASDs floating around in the codebase. My recommendation is to thus leave this PR unmerged for now, remaining as a low-priority issue, with the assumption that autocalibration is not required again until Peru. I believe @critcortex is most familiar with the autocalibration code so, if a chance to fix the optima_tb.asd references arises over the next few weeks, great. Otherwise, I will edit it myself when implementing design review edits. Of course, if autocalibration suddenly becomes necessary, we can expedite this merge as a temporary fix.

critcortex commented 7 years ago

I won't have a chance to fix until after Gauteng is finished, but that should be before we need autocalibrate for Peru, so that would be fine.

critcortex commented 7 years ago

I think it does highlight the need to have unit tests utilized as part of merging PRs, though ...

davidkedz commented 7 years ago

I'm definitely not against this and automating it as part of GitHub's testing set-up would be particularly great. Scheduling this, as always, is the tricky part.

critcortex commented 7 years ago

@davidkedz ah, this was the change in asd usage that I was referring to last week, that @hussainazfar introduced back in to obtain working functionality.

Given the concentration on autocalibrate, this may be a good spot to cover the common functionality of asd versions ...

davidkedz commented 7 years ago

@critcortex: Hai. It was the first one I checked when you floated the pruning intention and thus spurred me on to suggest caution in deleting branches. Either way, I will review it in more detail soon.

davidkedz commented 7 years ago

Base has been changed to gui-dev, which was already updated from this branch. No changes means that it's ok to merge and delete.