Closed willGraham01 closed 2 weeks ago
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?
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
:
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 :smile:
This sounds great Will, thanks. Speaking of getting opinion from those using the model let's tag @tbhallett
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.
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):
p
is currently scheduled to occupybed_type1
on2010-01-01
and2010-01-02
.2010-01-01
,p
is scheduled another appointment that imposes a footprint ofbed_type1
for 1 day, andbed_type2
for 6 days."Combining" the existing footprint with the incoming one, as is what is currently done, results in:
p
occupiesbed_type1
on2010-01-01
and2010-01-02
,p
occupiesbed_type2
on2010-01-03
through to2010-01-08
.However, there is no guarantee that
bed_type2
is available on2010-01-08
or2010-01-07
. The incoming footprint was allocated assuming the footprint would start on2010-01-01
, thus only reservingbed_type2
space until2010-01-06
. The existing footprint doesn't reserve anybed_type2
s. As a result, the facility can be over-allocated forbed_type2
on2010-01-08
and2010-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 ageneral_bed
on2010-07-04
for the rest of that day. However on2010-07-04
they are also given a new allocation for 7non_bed_space
days, to start immediately. The resulting combined footprint runs until2010-07-11
, however there is no guarantee that on2010-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 occupybed_type1
on2010-01-01
and2010-01-02
.p
occupiesbed_type2
from2010-01-03
through to2010-01-07
.Both the current and incoming footprints need
bed_type1
on2010-01-01
. On2010-01-02
, thebed_type2
requirement from the incoming footprint can be satisfied by assigning the "higher-priority" occupation ofbed_type1
from the current footprint. Then from2010-01-03
we allocate the remaining 5 days ofbed_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.