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

BedDays can over-allocate facility beds when combining footprints #1399

Closed
willGraham01 opened this issue Jun 6, 2024 · 4 comments · Fixed by #1437
Closed

BedDays can over-allocate facility beds when combining footprints #1399

willGraham01 opened this issue Jun 6, 2024 · 4 comments · Fixed by #1437
Labels
bug Something isn't working question Further information is requested

Comments

@willGraham01
Copy link
Collaborator

willGraham01 commented Jun 6, 2024

Whilst working on speedups & unit tests in #1392, I noticed the following behaviour in the model. Not sure if this is intended/accepted, but it struck me as odd so mentioning it here.

At present, when a person is already an inpatient and they are scheduled additional bed days at a facility, their remaining footprint is combined with the incoming footprint when allocating the total bed days they should be provided.

This can result in a facility being over-allocated in a particular type of bed. For example, consider the situation where (at some facility):

  • A person with ID p is currently scheduled to occupy bed_type1 on 2010-01-01 and 2010-01-02.
  • On 2010-01-01, p is scheduled another appointment that imposes a footprint of bed_type1 for 1 day, and bed_type2 for 6 days.

"Combining" the existing footprint with the incoming one, as is what is currently done, results in:

  • p occupies bed_type1 on 2010-01-01 and 2010-01-02,
  • p occupies bed_type2 on 2010-01-03 through to 2010-01-08.

However, there is no guarantee that bed_type2 is available on 2010-01-08 or 2010-01-07. The incoming footprint was allocated assuming the footprint would start on 2010-01-01, thus only reserving bed_type2 space until 2010-01-06. The existing footprint doesn't reserve any bed_type2s. As a result, the facility can be over-allocated for bed_type2 on 2010-01-08 and 2010-01-07. Since this logic applies to all HSI events, multiple patients can encounter the same issue, resulting in larger over-allocations on particular days.

Running the scale_run script for 1 year with an initial population of 10,000 will produce an example of this behaviour for patient 10107 on 2010-07-04. They occupy a general_bed on 2010-07-04 for the rest of that day. However on 2010-07-04 they are also given a new allocation for 7 non_bed_space days, to start immediately. The resulting combined footprint runs until 2010-07-11, however there is no guarantee that on 2010-07-11 there will be a bed for them.

Possible Solutions

If there is meant to be a "consistency check" for cases such as this, the impose_beddays_footprint function is too late to run it since bed days footprints are imposed after the HSI event has already run, so we cannot "undo" the HSI event that is in conflict. This check could be moved to happen earlier though, although I'm not sure on the exact location in the codebase as to where it should go.

Alternatively, it would be possible (at this stage in the code) to combine the two footprints by allocating the highest priority bed on any given day that they overlap, rather than extending the existing one. So in the case above;

  • p would occupy bed_type1 on 2010-01-01 and 2010-01-02.
  • p occupies bed_type2 from 2010-01-03 through to 2010-01-07.

Both the current and incoming footprints need bed_type1 on 2010-01-01. On 2010-01-02, the bed_type2 requirement from the incoming footprint can be satisfied by assigning the "higher-priority" occupation of bed_type1 from the current footprint.
Then from 2010-01-03 we allocate the remaining 5 days of bed_type2 from the incoming footprint.
This is guaranteed not to result in beds being over-allocated, since both footprints have checked for the availability of their respective beds, and we select the greatest of the two, rather than requesting both and allocating more bed-time without checking for it. However, it is a distinct behaviour from that which is currently implemented.

@willGraham01 willGraham01 added bug Something isn't working question Further information is requested labels Jun 6, 2024
@willGraham01 willGraham01 changed the title Beddays can over-allocate facility beds when combining footprints BedDays can over-allocate facility beds when combining footprints Jun 6, 2024
@mnjowe
Copy link
Collaborator

mnjowe commented Jul 9, 2024

Hi @willGraham01. Thanks for pointing out this important issue in bed-days.

On your first possible solution, I think the consistency check will need to happen in each disease module seeking bed days. That means the current bed days footprint implementation i.e. self.BEDDAYS_FOOTPRINT = self.make_beddays_footprint({'general_bed': 5}) will need to change to something like
self.BEDDAYS_FOOTPRINT = self.make_beddays_footprint({'general_bed': 5 - get_number_of_days_already_assigned_to_this_individual_on_this_bedtype(personal_id)}) or a loop that will find and subtract bed days per each bed type in both footprints(the newly created footprint and the already existing footprint). This is great in avoiding the conflicting bed days on bed types per individual BUT there will be repetition of code(to calculate bed days already imposed per each bed type) in every disease module seeking to use bed days.

The second possible solution seems good to me as it makes this implementation be done at one place hence avoiding repetition of code. The negative side of it is what you have already pointed out that its too late as the footprint has already being created and an HSI already run.

I think we can go by performance, that implementation which takes less time wins?

@willGraham01
Copy link
Collaborator Author

willGraham01 commented Jul 15, 2024

Thank you for the comments @mnjowe! It's been a month or two since I came across this, but after taking another look today to remind myself of what I was thinking:

For the 1st possible solution, I think we might be able to avoid repeating code in each of the disease modules if we edit the issue_bed_days_according_to_availability function. My understanding is that this is the function which is called before a HSI event runs to allocate beds to the event, and currently it ignores whether the person undergoing the HSI event is already allocated bed days. We could do something like this inside issue_bed_days_according_to_availability:

  • Check if the person undergoing the event is already an inpatient (so has bed days already scheduled)
  • Fetch the remaining footprint of the person in question, and temporarily remove it from the bed trackers.
  • Combine the footprint requested by the HSI event and the person's "remaining footprint".
  • Determine if the combined footprint can be allocated in the usual manner:
    • If it cannot be allocated, reinstate the person's old footprint. Since we only removed this temporarily, we always need to reinstate it (which should be fine, since this was allocated before we then tried to extend it).
    • If the combined footprint can be allocated, impose the combined footprint with the HSI event when it runs. (This will mean that some HSI events will appear to allocate more bed days than they should, since they'll be allocating the old footprint + the HSI event footprint in one go).

This solution I can see as being a slight slowdown as for every HSI event we'll have to check if the person is already an inpatient, fetch their footprint, combine the footprints, then assign the appropriate updated footprint. We'll be doing some of this even if we don't need to avoid this bug.

The 2nd solution is easier to implement as you say, since we can solve the problem whilst we're imposing the footprint rather than doing it beforehand. It should also be slightly faster (overwriting footprints rather than combining) execution-wise. I actually have an implementation of this solution in my BedDays rework branch. However, we would also need to get a second opinion from those who are using the model, since using the 2nd solution will change the behaviour of the model in a non-trivial manner (because this solution changes the bed-days allocation policy, which will change which people in the simulation do/do not receive treatment).

I'm happy to start working on a PR for either solution if you know which one you'd prefer 😄

@mnjowe
Copy link
Collaborator

mnjowe commented Jul 16, 2024

This sounds great Will, thanks. Speaking of getting opinion from those using the model let's tag @tbhallett

@tbhallett
Copy link
Collaborator

Thanks.
I think I've got my head around this now.
The second solution sounds good to me. I think this is the logic that was intended originally.

willGraham01 added a commit that referenced this issue Aug 20, 2024
* 1st pass writing combination method

* Write test that checks failure case in issue

* Expand on docstring for combining footprints method

* Force-cast to int when returning footprint since pd.datetime.timedelta doesn't know how to handle np.int64's

* Catch bug when determining priority on each day, write test to cover this case with a 3-bed types resolution

---------

Co-authored-by: Emmanuel Mnjowe <32415622+mnjowe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
Status: Issues
Development

Successfully merging a pull request may close this issue.

3 participants