Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates/fixes to MNH modules following on from Joe's PhD #1251

Merged
merged 166 commits into from
Feb 14, 2024

Conversation

joehcollins
Copy link
Collaborator

@joehcollins joehcollins commented Jan 2, 2024

This PR contains fixes/updates made across the suite of maternal/perinatal health models during the final stages of my PhD. This mainly includes fixes to small errors, updates to parameter values and updates to analysis scripts to run with new version of pandas.

In addition, i have now removed all of the logic in the model which is internally related to the squeeze factor in keeping with Margherita's work. I have rerun my own calibration scripts and everything seems fine.

To do:

  • Remove squeeze factor logic
  • Re-run MNH calibration scenario with squeeze factor removed
  • Update MNH calibration script
  • ?Update analysis scripts to run with new version of pandas
  • Flake8
  • Fix failing HS test

tdm32 and others added 30 commits November 11, 2022 11:07
Change tb mortality rate to 2011-2018 values (increased)
Add infant NVP
Change relative proportions of imported vs transmitted tb
Tb active case poll assigns cases within that month (not that year)
Same for importation event which runs monthly
Importation rate and transmission rate roughly calibrated
return blank appt footprint if child is not eligible
track repeat visits within event call
…nt interventions can be manipulated by scenario files. Also added fix which prevents higher coverage of AN inpatient care significantly increasing stillbirths
reset baseline active tb across 2010, transmission poll starts 2011
mdr importation rate 1.86% of this
…ysis with min/max availability of antenatal care
@marghe-molaro
Copy link
Collaborator

Hi @joehcollins, could you directly me to which test is failing? I can't seem to be able to see it currently as they're still running

@joehcollins
Copy link
Collaborator Author

Thanks @marghe-molaro its test_all_treatment_ids_defined_in_priority_policies

@marghe-molaro
Copy link
Collaborator

Luckily that should be an extremely easy fix!!

It basically means that in reviewing your modules you have added a treatment_ID that is currently not listed in the priority policies. So it needs to be added to the resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES.xlsx file. It should tell you which one is missing, otherwise can you think of any edits to treatment_ID names that you've made in this PR?

@joehcollins
Copy link
Collaborator Author

Luckily that should be an extremely easy fix!!

It basically means that in reviewing your modules you have added a treatment_ID that is currently not listed in the priority policies. So it needs to be added to the resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES.xlsx file. It should tell you which one is missing, otherwise can you think of any edits to treatment_ID names that you've made in this PR?

ok fab will do - will just wait for these tests to rerun and get back to you

@joehcollins
Copy link
Collaborator Author

@marghe-molaro - i do have a new treatment ID i forgot about. am i just adding a row to every sheet in that resource file for that new ID?

@marghe-molaro
Copy link
Collaborator

Hi @joehcollins, yes that's right, and add a relevant priority to them for each of the policies. If you have any doubt about which priority to assign them under a particular policy do reach out.

@joehcollins
Copy link
Collaborator Author

@marghe-molaro - adding in the missing treatment ID has fixed part of the error but the error still remains. Looks like its saying theres an additional treatment id that shouldnt be on the list? but that treatment ID is definitely used... You should be able to see on the error now

@marghe-molaro
Copy link
Collaborator

Hi @joehcollins, did you change the way in which the treatment_ID DeliveryCare_Comprehensive is set by any chance? It could be that it is now no longer picked up by the function during the cold read of the code

@joehcollins
Copy link
Collaborator Author

Hi @joehcollins, did you change the way in which the treatment_ID DeliveryCare_Comprehensive is set by any chance? It could be that it is now no longer picked up by the function during the cold read of the code

def __init__(self, module, person_id, timing, facility_level_of_this_hsi):
        super().__init__(module, person_id=person_id)
        assert isinstance(module, Labour)
       
        if timing == 'intrapartum':
            t_id = 'DeliveryCare_Comprehensive'
        else:
            t_id = 'PostnatalCare_Comprehensive'
        
        self.TREATMENT_ID = t_id
        self.EXPECTED_APPT_FOOTPRINT = self.make_appt_footprint({'MajorSurg': 1})
        self.ACCEPTED_FACILITY_LEVEL = facility_level_of_this_hsi
        self.timing = timing

Yep - this is now how the treatment ID is set up so thats likely related to the issue you describe above?

@marghe-molaro
Copy link
Collaborator

Unfortunately we don't have a "clean" way to deal with these exceptions - you can see how I addressed the other dynamically allocated treatment_IDs in the test ("Alri_Pneumonia_Treatment_Inpatient_Followup" and "Alri_Pneumonia_Treatment_Inpatient") and do the same for your treatment_ID

@joehcollins
Copy link
Collaborator Author

ahhh yes did see that for those - ok let me implement and then we should be all done!!

@tbhallett tbhallett requested review from marghe-molaro and removed request for tamuri and timcolbourn February 1, 2024 12:41
@tbhallett
Copy link
Collaborator

Adding @marghe-molaro to review for this, given the updating of the squeeze_factor logic.

@tbhallett
Copy link
Collaborator

Thanks @joehcollins

As far as I can tell (which isn't that far!), this all seems fine and it's got that you've abscised the squeeze_factor logic.

I think we should expedite putting this into master if this if the version that you're using for papers and things.

Copy link
Collaborator

@tamuri tamuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy for you to merge

@tbhallett tbhallett merged commit 3f1115b into master Feb 14, 2024
56 checks passed
@tbhallett tbhallett deleted the jcollins_mnh_module_updates_jan24 branch February 14, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants