Skip to content

Paramater Revamp - Measles#1653

Merged
tbhallett merged 39 commits intomasterfrom
1649-measles
Dec 8, 2025
Merged

Paramater Revamp - Measles#1653
tbhallett merged 39 commits intomasterfrom
1649-measles

Conversation

@mmsuarezcosta
Copy link
Copy Markdown
Collaborator

@mmsuarezcosta mmsuarezcosta commented Aug 4, 2025

Updating measles parameters (removing module hardcoding and adding labels)

To Note:

  • Deleted legacy beta.csv as it was not used anywhere in the code

Parameters to Review labeling:

  • Prob_severe: Marked as undetermined, but unsure if should be updated
  • Universal parameters (all): all are universal properties linked to references in the write-up, but 1) I did not go into the references to specifically determine a non-vague prior range, 2) I cannot confirm that they were not adjusted for calibration purposes.

Reference Notes:
No access to resource to get priors for beta and phase (https://pubmed.ncbi.nlm.nih.gov/18256664/ )
No access to resource to get priors for vaccine efficacy (Strebel, P. M., M. J. Papania, P. A. Gastañaduy and J. L. Goodson (2018). 37 - Measles Vaccines. Plotkin's Vaccines (Seventh Edition). S. A. Plotkin, W. A. Orenstein, P. A. Offit and K. M. Edwards, Elsevier: 579-618.e521)

@mmsuarezcosta mmsuarezcosta linked an issue Aug 4, 2025 that may be closed by this pull request
@mmsuarezcosta mmsuarezcosta changed the title 1649 measles Paramater Revamp - Measles Aug 13, 2025
@mmsuarezcosta mmsuarezcosta marked this pull request as ready for review August 15, 2025 22:22
@marghe-molaro
Copy link
Copy Markdown
Collaborator

Hi @mmsuarezcosta,
I think since it is describing the period over which we'd expect a single infection peak it should be labelled as local (there might be contexts in which we expect two infection peaks in a year, in which case the period should be 6 months). So this is not a structural assumption.
Notice that for the same reason the phase_shift should not be a structural assumption, since it describes in what month of the year we'd expect that peak (for Malawi it is assumed to be May, but it could be on another month in a different context)

@mmsuarezcosta
Copy link
Copy Markdown
Collaborator Author

@marghe-molaro Understood. I've changed the values as follows (please confirm this is OK)

  • period: prior_min = 6 months, prior_max = 24 months, prior_note = vague prior
  • phase_shift: prior_min = 0 months, prior_max = 23 months, prior_note = vague prior

@mmsuarezcosta mmsuarezcosta requested a review from mnjowe August 29, 2025 21:54
@mmsuarezcosta
Copy link
Copy Markdown
Collaborator Author

Ready for @mnjowe review

Copy link
Copy Markdown
Collaborator

@tbhallett tbhallett left a comment

Choose a reason for hiding this comment

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

labelling is fine in principle but the meta-data about the parameters (in symptoms.csv and cfr.csv is not being picked up -- this currently only happen when we called load_parameters_from_dataframe()).

Possible fixes would be:
(1) to manually put the parameters from those files into the parameter.csv so that it does work as intended.
(2) create some helper function to layer-on that metadata in a separate step. (I've proposed that for now)

Copy link
Copy Markdown
Collaborator

@tbhallett tbhallett left a comment

Choose a reason for hiding this comment

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

with that change, I can approve -- if you're happy with it.

Note that prior_min and prior_max for a dataframe could be overwritten to but it cumbersome!!

A mrore manual (but ultimately neater approach is really to simplify everthing into a few additional parameters in paraeter.csv (e.g. CFR_age_0_to_1, prob_rash_0_to_5, etc.)

Copy link
Copy Markdown
Collaborator

@mnjowe mnjowe left a comment

Choose a reason for hiding this comment

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

Thanks Mariana and Tim for your great work on this. Kindly find my comments below.

My question regarding parameters.csv concerns the relationship between the parameter label and its reference. I’ve noticed some parameters labeled as “local” but linked to references that are not Malawi-specific. Could you please clarify if this is intentional or acceptable?

As a minor concern, I’m a bit worried about having too many parameters even for smaller details such as dates or event frequencies in the resource file as this could potentially complicate the model’s adoption for new analysts

Comment thread src/tlo/core.py Outdated
Comment thread src/tlo/core.py Outdated
Comment thread src/tlo/core.py
Comment thread src/tlo/methods/measles.py
Comment thread src/tlo/methods/measles.py
Comment thread src/tlo/methods/measles.py Outdated
Comment thread src/tlo/methods/measles.py Outdated
Comment thread src/tlo/methods/measles.py
Comment thread src/tlo/methods/measles.py Outdated
@mmsuarezcosta
Copy link
Copy Markdown
Collaborator Author

Thanks Mariana and Tim for your great work on this. Kindly find my comments below.

My question regarding parameters.csv concerns the relationship between the parameter label and its reference. I’ve noticed some parameters labeled as “local” but linked to references that are not Malawi-specific. Could you please clarify if this is intentional or acceptable?

As a minor concern, I’m a bit worried about having too many parameters even for smaller details such as dates or event frequencies in the resource file as this could potentially complicate the model’s adoption for new analysts

Thank you @mnjowe for reviewing. Your review and feedback are extremely helpful. Thanks always for your level of detail in review.

  • Local references for universal params: This is OK. The goal in param labeling is to capture the essence of what the parameter is to be. So for example, a death rate may be expected to be universal and consistent regardless of context, yet it is cited from a local source. It is a grey area! If there are some that you believe should indeed be local that I have labeled as universal, please bring these forward so that we can discuss.
  • Too many parameters: Yes, this is definitely a big debate, and other developers have expressed similar concerns. For this project, we have erred on the side of 'over paramaterizing'. Can open this discussion again.

Please see complete documentation on guide for labeling HERE in tab Guide for Modelers that may support in ongoing revisions.

@mnjowe
Copy link
Copy Markdown
Collaborator

mnjowe commented Oct 23, 2025

On point number one I was asking about universal references for local params. Not sure if the explanation is the same

Thank you @mnjowe for reviewing. Your review and feedback are extremely helpful. Thanks always for your level of detail in review.

  • Local references for universal params: This is OK. The goal in param labeling is to capture the essence of what the parameter is to be. So for example, a death rate may be expected to be universal and consistent regardless of context, yet it is cited from a local source. It is a grey area! If there are some that you believe should indeed be local that I have labeled as universal, please bring these forward so that we can discuss.
  • Too many parameters: Yes, this is definitely a big debate, and other developers have expressed similar concerns. For this project, we have erred on the side of 'over paramaterizing'. Can open this discussion again.

Please see complete documentation on guide for labeling HERE in tab Guide for Modelers that may support in ongoing revisions.

@mmsuarezcosta
Copy link
Copy Markdown
Collaborator Author

On point number one I was asking about universal references for local params. Not sure if the explanation is the same

Thank you @mnjowe for reviewing. Your review and feedback are extremely helpful. Thanks always for your level of detail in review.

  • Local references for universal params: This is OK. The goal in param labeling is to capture the essence of what the parameter is to be. So for example, a death rate may be expected to be universal and consistent regardless of context, yet it is cited from a local source. It is a grey area! If there are some that you believe should indeed be local that I have labeled as universal, please bring these forward so that we can discuss.
  • Too many parameters: Yes, this is definitely a big debate, and other developers have expressed similar concerns. For this project, we have erred on the side of 'over paramaterizing'. Can open this discussion again.

Please see complete documentation on guide for labeling HERE in tab Guide for Modelers that may support in ongoing revisions.

Ah got it. Sorry for the misunderstanding. Yes, the point remains the same. We are mainly focused on capturing the ESSENCE of the parameter or what it WOULD BE in the perfect situation. Again, if there are any that you believe we should reconsider the labeling for, please do not hesitate to flag.

@mmsuarezcosta mmsuarezcosta requested a review from tdm32 November 9, 2025 21:14
@mmsuarezcosta
Copy link
Copy Markdown
Collaborator Author

Request for developer review @tdm32

@tbhallett
Copy link
Copy Markdown
Collaborator

@tdm32 -- does this look OK to you?
It seems good to me.

@tbhallett tbhallett merged commit b52f9df into master Dec 8, 2025
64 of 65 checks passed
@tbhallett tbhallett deleted the 1649-measles branch December 8, 2025 13:05
@tdm32
Copy link
Copy Markdown
Collaborator

tdm32 commented Dec 12, 2025

@tdm32 -- does this look OK to you? It seems good to me.

Sorry @tbhallett I'm late to respond, yes looks fine thank you.

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.

Measles Module

6 participants