Implement clinics in TLO by sangeetabhatia03 · Pull Request #1617 · UCL/TLOmodel

sangeetabhatia03 · 2025-05-08T13:20:47Z

No description provided.

sangeetabhatia03 · 2025-05-14T07:14:32Z

@marghe-molaro if my understanding is correct, the clinics object would be a nested dictionary - clinics = {Hiv: {Facility_id:daily_capability,...}, ...}
Can you check if this sounds correct?

sangeetabhatia03 · 2025-05-15T13:19:03Z

In the future, we might want to separate the rescaling of fungible/non-fungible capabilities. For now, only fungible time is rescaled. TODO: Discuss with group.

sangeetabhatia03 · 2025-05-20T23:26:19Z

hi @marghe-molaro as discussed, here is my plan for next steps:

Edit HSIEventQueueItem to include a property called clinic_access (boolean).
Define a getter function in HealthSystem:

def get_clinic_eligibility(str)
 """ Return true if str is one of the keys of the clinics dictionary"""

Edit _add_hsi_event_queue_item_to_hsi_event_queue as follows: before creating HSIEventQueueItem, check for clinic eligibility using hsi_event.module. That is

clinic_access = get_clinic_eligibility(hsi_event.module)

and set the value in HSIEvent appropriately.

marghe-molaro · 2025-05-21T08:03:23Z

Looks good @sangeetabhatia03 - in the last step ("and set the value in HSIEvent appropriately.") I think you mean HSIEventQueueItem

Signed-off-by: sangeetabhatia03 <sangeetabhatia03@gmail.com>

tbhallett · 2025-11-05T11:24:49Z

Checking calibration work flow: https://github.com/UCL/TLOmodel/actions/runs/19100393245

marghe-molaro

Hi @sangeetabhatia03,

Great job on this PR, I think looks basically ready!
I have pointed out a few things that need tidying up before it can be merged.
I would also add that the default ResourceFile_ClinicConfigurations doesn't really seem to require an "OtherClinic" clinic column.
(I would also vote to rename "OtherClinic", which sounds a bit strange in a scenario where to other clinics are present, to something like "GenericClinic".

We could have a chat about these comments over a meeting tomorrow?

marghe-molaro · 2025-11-04T11:52:09Z

            )
        )
-
+        self.eligible_clinic = health_system.get_clinic_eligibility(self)


I think the clinic eligibility should be an attribute of the HSIEventQueueItem, not of the HSI_Event itself (see original Issue #1617). This would highlight the fact that the eligibility of a given appointment for a given clinic is not an attribute of the HSI itself, but a policy level decision on how HSIs at a facility are handled (similar to priority), which in the model is always enforced through the HSIEventQueueItem class. We want to make sure that all the policy decisions can be dealt with centrally in the same location.

Notice that currently the clinic eligibility is defined both in HSI_Event and HSIEventQueueItem, which I think would create some confusion

Note: I realise currently priority is an attribute of the HSI_Event too, but it should be :)

This needs to be removed.

Commit 9e9c9d7 removes this.

marghe-molaro · 2025-11-04T11:59:50Z

        this does not demand officers that are _never_ available), and issue warning if not.
        """
-        if self.healthcare_system._officers_with_availability.issuperset(
+        if self.healthcare_system._officers_with_availability[self.eligible_clinic].issuperset(


If eligible_clinic were to be moved to HSIEventQueueEvent this check would have to be moved to when item is added to the queue

Margherita thinks _check_if_appt_footprint_can_run can be removed; I'll comment it out and proceed as it is not there and wait for Margherita's PR to remove this.

_check_if_appt_footprint_can_run has been removed, and I have also removed the duplication of clinic eligibility from hsi_event

marghe-molaro · 2025-11-04T16:42:18Z

+                level = self._facility_by_facility_id[facility_id].level
+                # Only rescale if rescaling factor is greater than 1 (i.e. don't reduce
+                # available capabilities if these were under-used the previous year).
+                rescaling_factor = self._summary_counter.frac_time_used_by_officer_type_and_level(


I think we need to break down the rescaling factor by clinic
(and actually, it would be good to do this by facility_id too, but this is beyond the scope of the PR)

Waiting for Margherita's PR to be merged.

After merging the latest PR that sets up rescaling factors by facility-id, the rescaling now happens by clinic and by facility id.

marghe-molaro · 2025-11-05T08:10:26Z

    def get_squeeze_factors(self, footprints_per_event, total_footprint, current_capabilities,
                            compute_squeeze_factor_to_district_level: bool
                            ):
+        ## TODO: check if there is a better place to intitalise this store


Mhmh I don't think this would be the best place to initialise, as it would alter our estimation of the squeeze factor, since we are effectively "resetting" the squeeze factor every time this function is called?
I would move it to just after the capabilities are initialised

Move it somehwere in pre_initialise_population

Moved to the end of pre_initialise_population as discussed.

marghe-molaro · 2025-11-05T08:28:55Z

                _priority = event.priority
                event = event.hsi_event
-                squeeze_factor = squeeze_factor_per_hsi_event[ev_num]                  # todo use zip here!
+                clinic = self.get_clinic_eligibility(event)


I think it would make more sense to call event.clinic_eligibility here, and not have to re-estimate this every time we go through the queue for every hsi.
Btw: I'm confused as to why the function self.get_clinic_eligibility works with event here (which is a HSIEventQueueItem) when it was defined as only accepting HSI_Event as an input (which here would be event.hsi_event)? Am I missing something?

i had not set up get_clinic_eligibility to expect HSIEventQueueItem; I think this code works because I only use the TREATMENT_ID property, which the passed object has. I will now amend get_clinic_eligibility to expect HSIEventQueueItem only.

Also addressed your original comment about using event.clinic_eligibility

marghe-molaro · 2025-11-05T09:56:49Z

        else:
            rand_queue = self.hsi_event_queue_counter

+        clinic_eligibility = self.get_clinic_eligibility(hsi_event)


I think it would be best to move this to schedule_hsi_event, and pass it as an argument to this function, since clinic_eligibility is an attribute of the HSIEventQueueItem itself

marghe-molaro · 2025-11-05T10:33:26Z

        assert all(2010 in sheet['year'].values for sheet in self.parameters['yearly_HR_scaling'].values())

+
+    def validate_clinic_configuration(self, clinic_capabilities_df: pd.DataFrame):


I think we also need to double check that the clinics defined in the two separate resource files match. Is this checked somewhere else atm?

Implemented this check via commit #89ab2f62576f9d6ead7372397913136df2627652

sangeetabhatia03 · 2025-11-11T14:05:56Z

Hi @sangeetabhatia03,

Great job on this PR, I think looks basically ready! I have pointed out a few things that need tidying up before it can be merged. I would also add that the default ResourceFile_ClinicConfigurations doesn't really seem to require an "OtherClinic" clinic column. (I would also vote to rename "OtherClinic", which sounds a bit strange in a scenario where to other clinics are present, to something like "GenericClinic".

We could have a chat about these comments over a meeting tomorrow?

Renamed OtherClinic to GenericClinic #e233df1b9bd429b0f5e2769ddcc7a61fccfb1903

…tent

…tQueueItem only

To avoid resetting it when squeeze factor is recoomputed

sangeetabhatia03 · 2025-11-24T23:33:11Z

@tbhallett this PR is now ready for your review.

tbhallett · 2025-11-27T12:41:04Z

Calibration workflow happening here: https://github.com/UCL/TLOmodel/actions/runs/19718098020

.... which all looks fine,

tbhallett

This is looking very good.

Just some very minor quibbles noted here, that should be very quick.

One important thing though: I do think that we would like to have a "wiki" page about this to explain the changes and also to give examples of how these new ResourceFile need to look (for whoever is going to create them).

so many changes due just to formatting -- I presume this was neccessary to overcome the ruff changes!!?

sangeetabhatia03 · 2025-12-04T16:50:17Z

Wiki page is here https://github.com/UCL/TLOmodel/wiki/Clinics-implementation-in-TLO

sangeetabhatia03 added 4 commits May 6, 2025 00:34

Dummy resource file for clinics and additional param in healthsystems

cf5ed0f

Necessary edits to docstring

dd0a193

Conditional read in new resource file

909e069

WIP; read and rescale clinic time

371ac5e

sangeetabhatia03 requested a review from marghe-molaro May 8, 2025 13:20

sangeetabhatia03 added 3 commits May 14, 2025 07:17

Update column name

ab18c46

Redefine daily capabilities down to non-fungible

d5b690d

Update daily capabilities based on fungible and non-fungible proportions

d576369

sangeetabhatia03 added 18 commits May 28, 2025 12:25

First pass at getting clinic eligibility

f5402f2

Fix syntax errors

4c82733

WIP; setter needs to be moved

2936a15

Moved getters and setters to healthsystem

d6b43eb

Fix typo

23163e2

Do not rescale if fractions don't add up to 1

f9c9297

Updated structure of Resource file

b809fd8

Modified Resource File and function to fill it out

224352e

Setup daily clinic capabilities for facility/cadre/module combination

4e337cb

Return fungible if module name not in Resource file

1eb5fd0

Clinic capabilities counter

e069635

Use appropriate counters

6361f32

Fix indentation error and add test (wip)

7c21176

Fix typos and further edits to test

41cceeb

Fix more typos while reading in resource file

838dba9

Update column name in resource file for consistency. Fix typos

e1ce587

Signed-off-by: sangeetabhatia03 <sangeetabhatia03@gmail.com>

Fix erroneous parens

329055d

Retrieve module columns from Resource file before formatting it

f3f77ff

marghe-molaro requested changes Nov 5, 2025

View reviewed changes

Rename OtherClinic to GenericClinic

e233df1

sangeetabhatia03 added 2 commits November 11, 2025 14:18

Rename OtherClinic to GenericClinic in resource files

1d31f1f

Check that clinic names across the two configuration files are consis…

89ab2f6

…tent

marghe-molaro mentioned this pull request Nov 13, 2025

Remove _check_if_appt_footprint_can_run #1751

Merged

sangeetabhatia03 added 4 commits November 13, 2025 14:18

Remove eligible_clinic property from hsi_event

9e9c9d7

Changes to ensure clinic eligibility is set and retrieved for HSIEven…

823efad

…tQueueItem only

Fix indentation

81ca231

Merge from master

88ae07e

sangeetabhatia03 marked this pull request as draft November 24, 2025 14:05

sangeetabhatia03 added 5 commits November 24, 2025 14:29

Use rescaling factor by facility id and clinic

83b4615

Iterate over clinics to get rescaling factor for each facility-id

e161778

Run through ruff

6e52d97

Some more formatting

c1ee2fe

Initialise squeeze factor store in pre_initialise_population

62d8dcc

To avoid resetting it when squeeze factor is recoomputed

sangeetabhatia03 marked this pull request as ready for review November 24, 2025 16:58

tbhallett requested changes Nov 28, 2025

View reviewed changes

marghe-molaro reviewed Nov 28, 2025

View reviewed changes

Comment thread src/tlo/methods/healthsystem.py Outdated

Make clinic_configuration and clinic_mapping an attribute

247c39c

tbhallett approved these changes Dec 4, 2025

View reviewed changes

marghe-molaro approved these changes Dec 4, 2025

View reviewed changes

tbhallett merged commit ca9458f into master Dec 4, 2025
64 of 65 checks passed

tbhallett deleted the fix-1608 branch December 4, 2025 17:24

tamuri mentioned this pull request Jan 9, 2026

Issue with change of HealthSystem parameters in master #1774

Closed

marghe-molaro mentioned this pull request Jan 14, 2026

Allow suspend/resume of simulations on Batch runs #1668

Merged

		assert all(2010 in sheet['year'].values for sheet in self.parameters['yearly_HR_scaling'].values())


		def validate_clinic_configuration(self, clinic_capabilities_df: pd.DataFrame):

Conversation

sangeetabhatia03 commented May 8, 2025

Uh oh!

sangeetabhatia03 commented May 14, 2025

Uh oh!

sangeetabhatia03 commented May 15, 2025

Uh oh!

sangeetabhatia03 commented May 20, 2025

Uh oh!

marghe-molaro commented May 21, 2025

Uh oh!

tbhallett commented Nov 5, 2025

Uh oh!

marghe-molaro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sangeetabhatia03 commented Nov 11, 2025

Uh oh!

sangeetabhatia03 commented Nov 24, 2025

Uh oh!

tbhallett commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbhallett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sangeetabhatia03 commented Dec 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

tbhallett commented Nov 27, 2025 •

edited

Loading