Conversation
|
Thanks for creating this PR @thewati. Happy to contribute |
…treatment ID color
…in priority_rank. Rework utils color map
…wati/diabetic_retinopathy
|
Hi @tbhallett and @mnjowe, |
|
Hi @thewati. Thanks for your great work on this PR. Before we start reviewing it, can you first look at the conflicts and see if you can resolve them? Thanks |
# Conflicts: # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/CVD.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/ClinicallyVulnerable.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Default.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/EHP_III.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/LCOA_EHP.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Naive.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/RMNCH.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Test Mode 1.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Test.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/VerticalProgrammes.csv
mnjowe
left a comment
There was a problem hiding this comment.
Hi @thewati,
Thank you again for the great work on the Diabetic Retinopathy PR. I’m still reviewing the tests and some other remaining files, but I thought it would be helpful to share my initial set of comments now to make tracking and follow-up easier.
Please feel free to question any of the comments or ask for clarification where needed. I’ll continue reviewing the remaining files and will share additional feedback if anything comes up.
Thanks.
| "rate_onset_to_mild_or_moderate_dr": Parameter(Types.REAL, | ||
| "Probability of people who get diagnosed with non-proliferative " | ||
| "mild/moderate diabetic retinopathy"), | ||
| "rate_mild_or_moderate_to_severe": Parameter(Types.REAL, | ||
| "Probability of people who get diagnosed with severe " | ||
| "diabetic retinopathy"), | ||
| "rate_severe_to_proliferative": Parameter(Types.REAL, | ||
| "Probability of people who get diagnosed with " | ||
| "proliferative diabetic retinopathy"), |
There was a problem hiding this comment.
It looks like you're using probabilities and not rates? if yes please change rate_ to prob_ i.e. rate_onset_to_mild_or_moderate_dr to prob_onset_to_mild_or_moderate_dr and so on
| "init_prob_any_dr": Parameter(Types.LIST, "Initial probability of anyone with diabetic retinopathy"), | ||
| "prob_any_dmo": Parameter(Types.LIST, "Probability of anyone with diabetic retinopathy having " | ||
| "Diabetic Macular Oedema (DMO)"), | ||
| "p_laser_success": Parameter(Types.REAL, "Diabetic retinopathy treatment/medication effectiveness"), |
There was a problem hiding this comment.
for consistency
| "p_laser_success": Parameter(Types.REAL, "Diabetic retinopathy treatment/medication effectiveness"), | |
| "prob_laser_success": Parameter(Types.REAL, "Diabetic retinopathy treatment/medication effectiveness"), |
| # "prob_diabetes_controlled": Parameter( | ||
| # Types.REAL, | ||
| # "Probability that a person with mild DR has controlled diabetes" | ||
| # ), | ||
| # "prob_mild_to_none_if_controlled_diabetes": Parameter( | ||
| # Types.REAL, | ||
| # "Probability that people with mild DR and controlled diabetes regress to 'none'" | ||
| # ), |
There was a problem hiding this comment.
I can't see these parameters in the resourcefile. If you have no plans to use them please delete them
There was a problem hiding this comment.
I kept this just in case Cova and Shaffi might want us to include this later. From the previous meetings we've had, the debate on this may not be over. It may also depend on whether they find data to inform regression (which is rare)
| Types.LIST, | ||
| "Probability matrix for vision status transitions for people with sight-threatening DR" | ||
| ), | ||
| "sensitivity_of_dilated_eye_exam_dr_dmo": Parameter( |
There was a problem hiding this comment.
Yes, I believe it is.
| "dr_status": Property( | ||
| Types.CATEGORICAL, | ||
| "DR status", | ||
| categories=["none", "mild_or_moderate", "severe", "proliferative"], |
There was a problem hiding this comment.
Just wondering, is there any reason we are using "mild_or_moderate" as a category instead of just choosing to go by one?
There was a problem hiding this comment.
Yes, I can't quite remember the exact reason. I think its something to do with the treatment of the two being the same if I am not mistaken. Probably something to ask Cova in the Epi call tomorrow
| if df.at[person_id, 'dmo_status'] == 'clinically_significant': | ||
| if self.module.rng.random_sample() < p['prob_repeat_dmo_treatment']: | ||
| treatment_for_cs_dmo = HSI_Dr_Dmo_AntiVEGF \ | ||
| if self.module.rng.random_sample() < 0.5 else HSI_Dr_Dmo_Focal_Laser |
There was a problem hiding this comment.
I think the 0.5 should be a parameter? I have seen it in a number of places an I'm not sure what it means
There was a problem hiding this comment.
So when a person has clinically_significant_macular_oedema (CSMO), the treatment given is either Anti-VEGF or Focal Laser as seen in the attached diagram. Right now, we don't know what proportion of people goes for each. So I just made it so that the chance of going for either is 0.5 (i.e., there's a 50% chance that a patient will have have to go for Anti-VEGF and a 50% chance that a patient will have to go for Focal Laser). Will make it a parameter.

There was a problem hiding this comment.
Great!
yes, please include it both in the parameters section and in the resource file. It will be updated once you agree on the right value
|
|
||
| will_progress = self.module.lm['onset_mild_or_moderate_dr'].predict(diabetes_and_alive_nodr, self.module.rng) | ||
| # will_progress_idx = will_progress[will_progress].index | ||
| will_progress_idx = df.index[np.where(will_progress)[0]] |
There was a problem hiding this comment.
This is a bug.
np.where(will_progress)[0] are positions from 0 to len(will_progress)-1 and you are applying such positions to df.index which is a full population dataframe. What follows is the selection of completely different people i.e. it picks rows at those positions in the full df. This result is tests like the below to fail
assert (df.loc[df.is_alive & ~df.nc_diabetes].dr_status == 'none').all()
Please use the commented block. its safe that way
will_progress_idx = will_progress[will_progress].index
Once you have done the above you may want to set to false squeeze_single_row_output argument in all your linear models. This ensures a single row is returned as a dataframe instead of just a boolean value. This practice also avoid the object has no index errors when doing will_progress_idx = will_progress[will_progress].index
see an example below on how you set squeeze single row to false
lm['onset_mild_or_moderate_dr'].predict(
diabetes_and_alive_nodr,
squeeze_single_row_output = False,
self.module.rng
)
| had_treatment_during_this_stage=had_treatment_during_this_stage, | ||
| diabetes_duration_greater_than_15_years=diabetes_duration_greater_than_15_years) | ||
| # moderate_to_severe_idx = moderate_to_severe[moderate_to_severe].index | ||
| mildmoderate_to_severe_idx = df.index[np.where(mildmoderate_to_severe)[0]] |
There was a problem hiding this comment.
same as my comment above, this is a bug. please use the commented code. you just need to update moderate_to_severe to mildmoderate_to_severe i.e.
moderate_to_severe_idx = mildmoderate_to_severe[mildmoderate_to_severe].index
| had_treatment_during_this_stage=had_treatment_during_this_stage, | ||
| diabetes_duration_greater_than_15_years=diabetes_duration_greater_than_15_years) | ||
| # severe_to_proliferative_idx = mild_to_moderate[severe_to_proliferative].index | ||
| severe_to_proliferative_idx = df.index[np.where(severe_to_proliferative)[0]] |
There was a problem hiding this comment.
same as my comment above. please use the commented out code. you just have to update it
severe_to_proliferative_idx = severe_to_proliferative[severe_to_proliferative].index
mnjowe
left a comment
There was a problem hiding this comment.
Hi @thewati,
Thank you again for the great work on the Diabetic Retinopathy PR. I’m still reviewing the tests and some other remaining files, but I thought it would be helpful to share my initial set of comments now to make tracking and follow-up easier.
Please feel free to question any of the comments or ask for clarification where needed. I’ll continue reviewing the remaining files and will share additional feedback if anything comes up.
Thanks.
|
Hi @mnjowe, |
# Conflicts: # resources/healthsystem/consumables/ResourceFile_Consumables_Items_and_Packages.csv # resources/healthsystem/infrastructure_and_equipment/ResourceFile_EquipmentCatalogue.csv # resources/healthsystem/infrastructure_and_equipment/ResourceFile_Equipment_Availability_Estimates.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/CVD.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/ClinicallyVulnerable.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Default.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/EHP_III.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/LCOA_EHP.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Naive.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/RMNCH.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Test Mode 1.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/Test.csv # resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES/VerticalProgrammes.csv
@thewati I hope this has now been addressed in the meeting yesterday |
Yes. |
Creating the DR module and associated files. Fixes #1595