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

Wasting Module #543

Open
wants to merge 104 commits into
base: master
Choose a base branch
from
Open

Wasting Module #543

wants to merge 104 commits into from

Conversation

tbhallett
Copy link
Collaborator

Fixes #542

@tbhallett tbhallett mentioned this pull request Apr 6, 2022
6 tasks
@tbhallett tbhallett added this to In progress in PR priorities via automation Apr 6, 2022
mnjowe and others added 25 commits October 5, 2023 09:01
…running without errors. added wasting analysis file to get more understanding of the current outputs before applying some major changes. added wasting resource file
# Conflicts:
#	src/tlo/methods/hsi_generic_first_appts.py
called the function nutritional_oedema_present within the WastingNaturalRecoveryEvent to reset the values for the oedema status based on WHZ>=-2
Removed progression to severe following treatment failure for MAM, at initialise population. These individuals will be called at the next polling event (in 3 months) to determine their outcome - recovery, remain, or death.
…lett/wasting_module

# Conflicts:
#	src/tlo/methods/wasting.py
# Conflicts:
#	resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES.xlsx
…lett/wasting_module

# Conflicts:
#	resources/ResourceFile_Wasting.xlsx
#	src/scripts/wasting_analyses/analysis_wasting.py
#	src/tlo/methods/hsi_generic_first_appts.py
#	src/tlo/methods/wasting.py
#	tests/test_wasting.py
@EvaJanouskova
Copy link
Collaborator

EvaJanouskova commented Jul 23, 2024

Hi @thewati, I can see you did some priority changes (commit cea20e0), here is a print screen of some changes made beside adding the undernutrition among the treatments in RF_PriorityRanking_ALLPOLICIES.xlsx, RMNCH sheet within this PR. (Because it is the xlsx file, I can't see which changes you made in the commit.)

RF_PriorityRanking_ALLPOLOCIES_RMNCH_changes_wasting_module

Are these changes suggested by you? Are we still going to make these changes? I'm not sure who and why suggested that. Red is current version in master, green is what it is suggested to be changed to in this PR.

@EvaJanouskova
Copy link
Collaborator

@thewati, here is the version from the old master, which is the starting point of this branch:
ResourceFile_PriorityRanking_ALLPOLICIES_wasting_module_masterbase.xlsx

And here is the version from this branch:
ResourceFile_PriorityRanking_ALLPOLICIES_wasting_module.xlsx

You can review all suggested changes by comparing these versions.

# Conflicts:
#	resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES.xlsx
#	src/tlo/methods/hsi_generic_first_appts.py
# Conflicts:
#	resources/healthsystem/priority_policies/ResourceFile_PriorityRanking_ALLPOLICIES.xlsx
@EvaJanouskova
Copy link
Collaborator

EvaJanouskova commented Jul 23, 2024

In terms of discussion today in stand-up call about how to resolve merge conflict due to changes in generic first appointment HSI events ....

Thanks a lot, @matt-graham! This is really helpful!

Hi @matt-graham, the Documentation check fails:
File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line 33, in <module> class Wasting(Module, GenericFirstAppointmentsMixin): File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line [55](https://github.com/UCL/TLOmodel/actions/runs/10066494565/job/27827994771?pr=543#step:6:56)2, in Wasting individual_properties: IndividualProperties, ^^^^^^^^^^^^^^^^^^^^ NameError: name 'IndividualProperties' is not defined

I tried to define it the same way as it is in stunting, but it doesn't seem to be working. Shouldn't the TYPE_CHECKING be turned on for wasting somewhere, or smt like that?

…dule

# Conflicts:
#	resources/ResourceFile_Wasting.csv
#	src/tlo/methods/wasting.py
@thewati
Copy link
Collaborator

thewati commented Jul 24, 2024

Hi @thewati, I can see you did some priority changes (commit cea20e0), here is a print screen of some changes made beside adding the undernutrition among the treatments in RF_PriorityRanking_ALLPOLICIES.xlsx, RMNCH sheet within this PR. (Because it is the xlsx file, I can't see which changes you made in the commit.)

RF_PriorityRanking_ALLPOLOCIES_RMNCH_changes_wasting_module

Are these changes suggested by you? Are we still going to make these changes? I'm not sure who and why suggested that. Red is current version in master, green is what it is suggested to be changed to in this PR.

Hi @EvaJanouskova , I had a look at both files and I remember that commit. In that commit, I believe we added 3 treatment IDs called Undernutrition_Feeding_Supplementary, Undernutrition_Feeding_Inpatient, and Undernutrition_Feeding_Outpatient in the Naive sheet. I am not sure who changed or suggested the changes to the priorities. That commit did not touch the priorities you have highlighted. @mnjowe would you have an idea on the priorities Eva has highlighted?

@matt-graham
Copy link
Collaborator

In terms of discussion today in stand-up call about how to resolve merge conflict due to changes in generic first appointment HSI events ....

Thanks a lot, @matt-graham! This is really helpful!

Hi @matt-graham, the Documentation check fails: File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line 33, in <module> class Wasting(Module, GenericFirstAppointmentsMixin): File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line [55](https://github.com/UCL/TLOmodel/actions/runs/10066494565/job/27827994771?pr=543#step:6:56)2, in Wasting individual_properties: IndividualProperties, ^^^^^^^^^^^^^^^^^^^^ NameError: name 'IndividualProperties' is not defined

I tried to define it the same way as it is in stunting, but it doesn't seem to be working. Shouldn't the TYPE_CHECKING be turned on for wasting somewhere, or smt like that?

Ah that's because you need to import IndividualProperties from tlo.population - as this is only required for type hint you can do within the if TYPE_CHECKING block

@EvaJanouskova
Copy link
Collaborator

EvaJanouskova commented Jul 24, 2024

In terms of discussion today in stand-up call about how to resolve merge conflict due to changes in generic first appointment HSI events ....

Thanks a lot, @matt-graham! This is really helpful!

Hi @matt-graham, the Documentation check fails: File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line 33, in <module> class Wasting(Module, GenericFirstAppointmentsMixin): File "/home/runner/work/TLOmodel/TLOmodel/.tox/docs/lib/python3.11/site-packages/tlo/methods/wasting.py", line [55](https://github.com/UCL/TLOmodel/actions/runs/10066494565/job/27827994771?pr=543#step:6:56)2, in Wasting individual_properties: IndividualProperties, ^^^^^^^^^^^^^^^^^^^^ NameError: name 'IndividualProperties' is not defined
I tried to define it the same way as it is in stunting, but it doesn't seem to be working. Shouldn't the TYPE_CHECKING be turned on for wasting somewhere, or smt like that?

Ah that's because you need to import IndividualProperties from tlo.population - as this is only required for type hint you can do within the if TYPE_CHECKING block

@matt-graham, The following is in the code:

if TYPE_CHECKING:
    from tlo.methods.hsi_generic_first_appts import HSIEventScheduler
    from tlo.population import IndividualProperties

Do you mean smt else?

…separates individual columns); wast: minor updates in comments
@matt-graham
Copy link
Collaborator

@matt-graham, The following is in the code:

if TYPE_CHECKING:
    from tlo.methods.hsi_generic_first_appts import HSIEventScheduler
    from tlo.population import IndividualProperties

Do you mean smt else?

Sorry I missed that you already had the import in the code. It looks like the issue is actually that you are missing a

from __future__ import annotations

(see here for explanation of why required).
line at top of file

@EvaJanouskova
Copy link
Collaborator

EvaJanouskova commented Jul 24, 2024

@matt-graham, The following is in the code:

if TYPE_CHECKING:
    from tlo.methods.hsi_generic_first_appts import HSIEventScheduler
    from tlo.population import IndividualProperties

Do you mean smt else?

Sorry I missed that you already had the import in the code. It looks like the issue is actually that you are missing a

from __future__ import annotations

(see here for explanation of why required). line at top of file

Yes, this seems as what could cause the errors! Thank you @matt-graham!!

Great explanation. It's a cool trick you use there.

@mnjowe
Copy link
Collaborator

mnjowe commented Jul 25, 2024

Hi @thewati, I can see you did some priority changes (commit cea20e0), here is a print screen of some changes made beside adding the undernutrition among the treatments in RF_PriorityRanking_ALLPOLICIES.xlsx, RMNCH sheet within this PR. (Because it is the xlsx file, I can't see which changes you made in the commit.)
RF_PriorityRanking_ALLPOLOCIES_RMNCH_changes_wasting_module
Are these changes suggested by you? Are we still going to make these changes? I'm not sure who and why suggested that. Red is current version in master, green is what it is suggested to be changed to in this PR.

Hi @EvaJanouskova , I had a look at both files and I remember that commit. In that commit, I believe we added 3 treatment IDs called Undernutrition_Feeding_Supplementary, Undernutrition_Feeding_Inpatient, and Undernutrition_Feeding_Outpatient in the Naive sheet. I am not sure who changed or suggested the changes to the priorities. That commit did not touch the priorities you have highlighted. @mnjowe would you have an idea on the priorities Eva has highlighted?

I agree to your point @thewati. In this PR we were aiming at adding treatment IDs from wasting module only. @EvaJanouskova if there are merge conflicts that require manual way of merging commits I can help. I guess we just need to copy original file from master and manually add the three treatment IDs @thewati has mentioned above. Thanks

@EvaJanouskova
Copy link
Collaborator

@marghe-molaro, Could you please confirm the updates in Priorities?

There are 3 new lines added in all sheets:

  • Undernutrition_Feeding_Supplementary
  • Undernutrition_Feeding_Inpatient
  • Undernutrition_Feeding_Outpatient

The values are always same for all 3 lines.
In most sheets the values are:

Priority FT_if_5orUnder FT_if_pregnant FT_if_tbdiagnosed FT_if_Hivdiagnosed
0 -1 -1 -1 -1

The only exception is the Naive sheet, where the values are:

Priority FT_if_5orUnder FT_if_pregnant FT_if_tbdiagnosed FT_if_Hivdiagnosed
2 -1 -1 -1 -1

Are these fine with you?

@marghe-molaro
Copy link
Collaborator

Hi @EvaJanouskova,

For the LCOA_EHP and EHP_III you would need to double check with @sakshimohan.

The Naive values look correct, but not the ones for the other sheets.

For "Default" and "Test" and "Test Mode 1" you can go for (0, -1, -1, -1, -1).
In the case of "RMNCH" I believe these may need to be prioritised as mainly concerning under 5 care (@tbhallett should confirm this) in which case I would go for (5, 3, -1, -1, -1)
In the case of ClinicallyVulnerable, go for (3, 2, 2, 2, 2)
For VerticalProgrammes and CVD you can go for (3, -1, -1, -1, -1)

@EvaJanouskova
Copy link
Collaborator

Hi @EvaJanouskova,

For the LCOA_EHP and EHP_III you would need to double check with @sakshimohan.

The Naive values look correct, but not the ones for the other sheets.

For "Default" and "Test" and "Test Mode 1" you can go for (0, -1, -1, -1, -1). In the case of "RMNCH" I believe these may need to be prioritised as mainly concerning under 5 care (@tbhallett should confirm this) in which case I would go for (5, 3, -1, -1, -1) In the case of ClinicallyVulnerable, go for (3, 2, 2, 2, 2) For VerticalProgrammes and CVD you can go for (3, -1, -1, -1, -1)

Thanks @marghe-molaro. @sakshimohan and @tbhallett, could you please confirm the above? Thank you!

logger.debug(key='debug', data="consumables not available, so can't use it.")

def did_not_run(self):
logger.debug("HSI_Undernutrition_Feeding_Outpatient: did not run")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Determinism test is failing because this uses the old (and now removed) way of logging. log needs arguments key and data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's just delete this obsolete line then!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tamuri, Why the test_determinism is not failing on my computer?

@sakshimohan
Copy link
Collaborator

Hi @EvaJanouskova,
For the LCOA_EHP and EHP_III you would need to double check with @sakshimohan.
The Naive values look correct, but not the ones for the other sheets.
For "Default" and "Test" and "Test Mode 1" you can go for (0, -1, -1, -1, -1). In the case of "RMNCH" I believe these may need to be prioritised as mainly concerning under 5 care (@tbhallett should confirm this) in which case I would go for (5, 3, -1, -1, -1) In the case of ClinicallyVulnerable, go for (3, 2, 2, 2, 2) For VerticalProgrammes and CVD you can go for (3, -1, -1, -1, -1)

Thanks @marghe-molaro. @sakshimohan and @tbhallett, could you please confirm the above? Thank you!

Hi @EvaJanouskova . Is there a more detailed description of the three interventions which I could refer to, to assign them the right priority?

@EvaJanouskova
Copy link
Collaborator

EvaJanouskova commented Jul 27, 2024

Hi @EvaJanouskova,
For the LCOA_EHP and EHP_III you would need to double check with @sakshimohan.
The Naive values look correct, but not the ones for the other sheets.
For "Default" and "Test" and "Test Mode 1" you can go for (0, -1, -1, -1, -1). In the case of "RMNCH" I believe these may need to be prioritised as mainly concerning under 5 care (@tbhallett should confirm this) in which case I would go for (5, 3, -1, -1, -1) In the case of ClinicallyVulnerable, go for (3, 2, 2, 2, 2) For VerticalProgrammes and CVD you can go for (3, -1, -1, -1, -1)

Thanks @marghe-molaro. @sakshimohan and @tbhallett, could you please confirm the above? Thank you!

Hi @EvaJanouskova . Is there a more detailed description of the three interventions which I could refer to, to assign them the right priority?

@sakshimohan, See the description in the write-up, page 8.

@EvaJanouskova
Copy link
Collaborator

@tbhallett, We may need to include some equipment for the wasting module. Would it be fine to address this later, or do you need it for another project asap, in which case I should incorporate any equipment that comes to mind at this moment within this PR?

@tbhallett
Copy link
Collaborator Author

@tbhallett, We may need to include some equipment for the wasting module. Would it be fine to address this later, or do you need it for another project asap, in which case I should incorporate any equipment that comes to mind at this moment within this PR?

Thanks Eva. It seems a good idea to me to add in the declarations now, if they are coming to mind. You can always add more later if it turns out you missed some. We are going to run the full model for the costing calculations with this module included soon, so I would say please include things if you can.

@sakshimohan
Copy link
Collaborator

(3, -1, -1, -1, -1)

Thanks @EvaJanouskova.

For EHP, you can go for (2, -1, -1, -1, -1) for all three treatment_ID s.
For LCOA:

  • (2, -1, -1, -1, -1) for Undernutrition_Feeding_Supplementary, and Undernutrition_Feeding_Outpatient;
  • (4, -1, -1, -1, -1) for Undernutrition_Feeding_Inpatient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PR priorities
Ready for EM review
Development

Successfully merging this pull request may close these issues.

Wasting Outputs Complete the Wasting module
9 participants