Create date_onset Property in cardio_metabolic_disorders.py#1849
Create date_onset Property in cardio_metabolic_disorders.py#1849
Conversation
|
I thought it would be better to just capture the date_onset of all conditions. Then I could just access |
|
Hi @mnjowe and @tbhallett, |
|
This is well noted @thewati. I will take a look |
|
Hi @thewati. Looking at the use case of this PR, I think adding a property that captures date_onset of all condition will not be a good idea. The model is already heavy and I think we should exhaust all possible alternatives before we decide on introducing a new property. Also I think adding this date_onset property to all conditions while we are interested only in one condition will accumulate some unnecessary computational costs In this case, how about we create a dictionary that maps diabetes individual Ids to their date of diabetes onset? This should only look into diabetes as we are not interested in any other conditions. That way, if you capture their date of onset in Diabetic Retinopathy you can delete the keys in the diabetes dictionary to make it as smaller as possible. What do you think? @tbhallett any thoughts? |
Thanks for pointing this out. I had coded it this way so that anybody can use it in future if needed and also to maintain consistency... But I agree, this will cause unnecessary overhead. I can change this to a dictionary that only stores the date of onset as you suggested |
|
@mnjowe this is now using a dictionary. I'm done |
| if condition == 'nc_diabetes': | ||
| for person_id in idx_acquires_condition: | ||
| if person_id not in self.module.diabetes_onset_dates: | ||
| self.module.diabetes_onset_dates[person_id] = self.sim.date | ||
|
|
There was a problem hiding this comment.
This is totally fine and very readable. I was thinking that a more pythonic version would be as follows, but this is perhaps less readable (now that I read it after writing it out)
if condition == 'nc_diabetes':
self.module.diabetes_onset_dates |= {
person_id: self.sim.date
for person_id in idx_acquires_condition
if person_id not in self.module.diabetes_onset_dates
}|
Hi @tbhallett,
Can this be undone? |
|
You could raise a new PR to make amendments if needed.
Sent from Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Watipaso Mulwafu ***@***.***>
Sent: Tuesday, April 21, 2026 1:22:10 PM
To: UCL/TLOmodel ***@***.***>
Cc: Hallett, Timothy B ***@***.***>; Mention ***@***.***>
Subject: Re: [UCL/TLOmodel] Create date_onset Property in cardio_metabolic_disorders.py (PR #1849)
CAUTION: This message came from outside Imperial. Do not click links or open attachments unless you recognise the sender and were expecting this email.
[https://avatars.githubusercontent.com/u/39279950?s=20&v=4]thewati left a comment (UCL/TLOmodel#1849)<#1849?email_source=notifications&email_token=AJRDOFGDCCORAFVIJYAPDQ34W5RXFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRYHA2DOOBRG43KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4288478176>
Hi @tbhallett<https://github.com/tbhallett>,
Sorry I was a bit clumsy. I noticed I missed 2 things in this PR.
1. At line 1065, I accidentally used if condition == 'nc_diabetes': instead of if condition == 'diabetes':. This is something that changed with one of the rollbacks I did.
2. Somewhere around line 450 (inside initialise_pupulation), I need to add the code below to capture onset dates for initial diabetes prevalence. There was a mismatch between those with diabetes and those with onset dates and this is coming from the the part where I missed this in prevalence:
`if condition == 'diabetes':
has_diabetes = df.index[df.is_alive & df.nc_diabetes]
for person_id in has_diabetes:
if person_id not in self.diabetes_onset_dates:
self.diabetes_onset_dates[person_id] = self.sim.date`
Can this be undone?
—
Reply to this email directly, view it on GitHub<#1849?email_source=notifications&email_token=AJRDOFGDCCORAFVIJYAPDQ34W5RXFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRYHA2DOOBRG43KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4288478176>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJRDOFB3TBVUXUCWZTRRAG34W5RXFAVCNFSM6AAAAACWYARPE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEOBYGQ3TQMJXGY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Creating date_onset property for conditions in the cardio_metabolic_disorders module. Fixes issue #1845. The nc_diabetes_date_onset property will be used in issue #1595