From 364250a2917dfe61ea7b63bb096a448de2d03495 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 6 Jan 2023 13:13:58 -0400 Subject: [PATCH] feat: Generate a partial schedule when there are more sessions than timeslots (#4950) * refactor: move session/timeslot selection for sched editor to querysets Purpose here is to make it easier to reuse the session and timeslot selection logic between the schedule editor and the schedule generator. Additionally resolves a todo-list item to unify the list of TimeSlotType ids in the IGNORE_TIMESLOT_TYPES tuple and the SessionQuerySet.requests() method. * refactor: use new helpers to select sessions/slots for sched generator * refactor: eliminate some code lint * feat: Split sched gen TimeSlot into scheduled/unscheduled variants (work in progress) * feat: First pass at supporting unscheduled timeslots (work in progress) * feat: Handle unscheduled timeslots in make_capacity_adjustments() (work in progress) * feat: Handle unscheduled timeslots in time-relation constraint check (work in progress) * feat: Reflect unsched timeslots in messages from by schedule generator * fix: Prevent exception in pretty_print() if base schedule not assigned * refactor: Avoid flood of time relation constraint warning messages * test: update test_too_many_sessions --- .../management/commands/generate_schedule.py | 207 ++++++++++++------ ietf/meeting/models.py | 21 +- ietf/meeting/tests_schedule_generator.py | 10 +- ietf/meeting/views.py | 19 +- 4 files changed, 171 insertions(+), 86 deletions(-) diff --git a/ietf/meeting/management/commands/generate_schedule.py b/ietf/meeting/management/commands/generate_schedule.py index ade11ef08..c0645eed6 100644 --- a/ietf/meeting/management/commands/generate_schedule.py +++ b/ietf/meeting/management/commands/generate_schedule.py @@ -14,6 +14,7 @@ import time from collections import defaultdict from functools import lru_cache from typing import NamedTuple, Optional +from warnings import warn from django.contrib.humanize.templatetags.humanize import intcomma from django.core.management.base import BaseCommand, CommandError @@ -170,9 +171,7 @@ class ScheduleHandler(object): * sunday timeslots * timeslots used by the base schedule, if any """ - # n.b., models.TimeSlot is not the same as TimeSlot! - timeslots_db = models.TimeSlot.objects.filter( - meeting=self.meeting, + timeslots_db = self.meeting.timeslot_set.that_can_be_scheduled().filter( type_id='regular', ).exclude( location__capacity=None, @@ -183,10 +182,9 @@ class ScheduleHandler(object): else: fixed_timeslots = timeslots_db.filter(pk__in=self.base_schedule.qs_timeslots_in_use()) free_timeslots = timeslots_db.exclude(pk__in=fixed_timeslots) - - timeslots = {TimeSlot(t, self.verbosity) for t in free_timeslots.select_related('location')} + timeslots = {DatatrackerTimeSlot(t, verbosity=self.verbosity) for t in free_timeslots.select_related('location')} timeslots.update( - TimeSlot(t, self.verbosity, is_fixed=True) for t in fixed_timeslots.select_related('location') + DatatrackerTimeSlot(t, verbosity=self.verbosity, is_fixed=True) for t in fixed_timeslots.select_related('location') ) return {t for t in timeslots if t.day != 'sunday'} @@ -195,12 +193,7 @@ class ScheduleHandler(object): Extra arguments are passed to the Session constructor. """ - sessions_db = models.Session.objects.filter( - meeting=self.meeting, - type_id='regular', - schedulingevent__status_id='schedw', - ) - + sessions_db = self.meeting.session_set.that_can_be_scheduled().filter(type_id='regular') if self.base_schedule is None: fixed_sessions = models.Session.objects.none() else: @@ -225,16 +218,18 @@ class ScheduleHandler(object): for bc in models.BusinessConstraint.objects.all() } - timeslots = self._available_timeslots() - for timeslot in timeslots: - timeslot.store_relations(timeslots) - sessions = self._sessions_to_schedule(business_constraint_costs, self.verbosity) for session in sessions: # The complexity of a session also depends on how many # sessions have declared a conflict towards this session. session.update_complexity(sessions) + timeslots = self._available_timeslots() + for _ in range(len(sessions) - len(timeslots)): + timeslots.add(GeneratorTimeSlot(verbosity=self.verbosity)) + for timeslot in timeslots: + timeslot.store_relations(timeslots) + self.schedule = Schedule( self.stdout, timeslots, @@ -270,9 +265,9 @@ class Schedule(object): def __str__(self): return 'Schedule ({} timeslots, {} sessions, {} scheduled, {} in base schedule)'.format( - len(self.timeslots), + sum(1 for ts in self.timeslots if ts.is_scheduled), len(self.sessions), - len(self.schedule), + sum(1 for ts in self.schedule if ts.is_scheduled), len(self.base_schedule) if self.base_schedule else 0, ) @@ -280,9 +275,13 @@ class Schedule(object): """Pretty print the schedule""" last_day = None sched = dict(self.schedule) - if include_base: + if include_base and self.base_schedule is not None: sched.update(self.base_schedule) - for slot in sorted(sched, key=lambda ts: ts.start): + timeslots = {'scheduled': [], 'unscheduled': []} + for ts in self.timeslots: + timeslots['scheduled' if ts.is_scheduled else 'unscheduled'].append(ts) + + for slot in sorted(timeslots['scheduled'], key=lambda ts: ts.start): if last_day != slot.start.date(): last_day = slot.start.date() print(""" @@ -293,9 +292,16 @@ class Schedule(object): print('{}: {}{}'.format( models.TimeSlot.objects.get(pk=slot.timeslot_pk), models.Session.objects.get(pk=sched[slot].session_pk), - ' [BASE]' if slot in self.base_schedule else '', + ' [BASE]' if self.base_schedule and slot in self.base_schedule else '', )) + print(""" +----------------- + Unscheduled +-----------------""") + for slot in timeslots['unscheduled']: + print(' * {}'.format(models.Session.objects.get(pk=sched[slot].session_pk))) + @property def fixed_cost(self): return sum(self._fixed_costs.values()) @@ -328,12 +334,13 @@ class Schedule(object): def save_assignments(self, schedule_db): for timeslot, session in self.schedule.items(): - models.SchedTimeSessAssignment.objects.create( - timeslot_id=timeslot.timeslot_pk, - session_id=session.session_pk, - schedule=schedule_db, - badness=session.last_cost, - ) + if timeslot.is_scheduled: + models.SchedTimeSessAssignment.objects.create( + timeslot_id=timeslot.timeslot_pk, + session_id=session.session_pk, + schedule=schedule_db, + badness=session.last_cost, + ) def adjust_for_timeslot_availability(self): """ @@ -352,9 +359,15 @@ class Schedule(object): .format(num_to_schedule, num_free_timeslots)) def make_capacity_adjustments(t_attr, s_attr): - availables = [getattr(timeslot, t_attr) for timeslot in self.free_timeslots] + availables = [getattr(timeslot, t_attr) for timeslot in self.free_timeslots if timeslot.is_scheduled] availables.sort() sessions = sorted(self.free_sessions, key=lambda s: getattr(s, s_attr), reverse=True) + # If we have timeslots with is_scheduled == False, len(availables) may be < len(sessions). + # Add duplicates of the largest capacity timeslot to make up the difference. This will + # still report violations where there is *no* timeslot available that accommodates a session + # but will not check that there are *enough* timeslots for the number of long sessions. + n_unscheduled = len(sessions) - len(availables) + availables.extend([availables[-1]] * n_unscheduled) # availables is sorted, so [-1] is max violations, cost = [], 0 for session in sessions: found_fit = False @@ -446,9 +459,16 @@ class Schedule(object): For initial scheduling, it is not a hard requirement that the timeslot is long or large enough, though that will be preferred due to the lower cost. """ + n_free_sessions = len(list(self.free_sessions)) + n_free_scheduled_slots = sum(1 for sl in self.free_timeslots if sl.is_scheduled) if self.verbosity >= 2: - self.stdout.write('== Initial scheduler starting, scheduling {} sessions in {} timeslots ==' - .format(len(list(self.free_sessions)), len(list(self.free_timeslots)))) + self.stdout.write( + f'== Initial scheduler starting, scheduling {n_free_sessions} sessions in {n_free_scheduled_slots} timeslots ==' + ) + if self.verbosity >= 1 and n_free_sessions > n_free_scheduled_slots: + self.stdout.write( + f'WARNING: fewer timeslots ({n_free_scheduled_slots}) than sessions ({n_free_sessions}). Some sessions will not be scheduled.' + ) sessions = sorted(self.free_sessions, key=lambda s: s.complexity, reverse=True) for session in sessions: @@ -458,14 +478,20 @@ class Schedule(object): def timeslot_preference(t): proposed_schedule = self.schedule.copy() proposed_schedule[t] = session - return self.calculate_dynamic_cost(proposed_schedule)[1], t.duration, t.capacity - + return ( + self.calculate_dynamic_cost(proposed_schedule)[1], + t.duration if t.is_scheduled else datetime.timedelta(hours=1000), # unscheduled slots sort to the end + t.capacity if t.is_scheduled else math.inf, # unscheduled slots sort to the end + ) possible_slots.sort(key=timeslot_preference) self._schedule_session(session, possible_slots[0]) if self.verbosity >= 3: - self.stdout.write('Scheduled {} at {} in location {}' - .format(session.group, possible_slots[0].start, - possible_slots[0].location_pk)) + if possible_slots[0].is_scheduled: + self.stdout.write('Scheduled {} at {} in location {}' + .format(session.group, possible_slots[0].start, + possible_slots[0].location_pk)) + else: + self.stdout.write('Scheduled {} in unscheduled slot') def optimise_schedule(self): """ @@ -487,9 +513,10 @@ class Schedule(object): best_cost = math.inf shuffle_next_run = False last_run_cost = None - switched_with = None - - for run_count in range(1, self.max_cycles+1): + run_count = 0 + + for _ in range(self.max_cycles): + run_count += 1 items = list(self.schedule.items()) random.shuffle(items) @@ -585,7 +612,7 @@ class Schedule(object): """ optimised_timeslots = set() for timeslot in list(self.schedule.keys()): - if timeslot in optimised_timeslots or timeslot.is_fixed: + if timeslot in optimised_timeslots or timeslot.is_fixed or not timeslot.is_scheduled: continue timeslot_overlaps = sorted(timeslot.full_overlaps, key=lambda t: t.capacity, reverse=True) sessions_overlaps = [self.schedule.get(t) for t in timeslot_overlaps] @@ -629,7 +656,7 @@ class Schedule(object): del proposed_schedule[timeslot1] return self.calculate_dynamic_cost(proposed_schedule)[1] - def _switch_sessions(self, timeslot1, timeslot2): + def _switch_sessions(self, timeslot1, timeslot2) -> Optional['Session']: """ Switch the sessions currently in timeslot1 and timeslot2. If timeslot2 had a session scheduled, returns that Session instance. @@ -637,11 +664,11 @@ class Schedule(object): session1 = self.schedule.get(timeslot1) session2 = self.schedule.get(timeslot2) if timeslot1 == timeslot2: - return False + return None if session1 and not session1.fits_in_timeslot(timeslot2): - return False + return None if session2 and not session2.fits_in_timeslot(timeslot1): - return False + return None if session1: self.schedule[timeslot2] = session1 elif session2: @@ -658,15 +685,43 @@ class Schedule(object): self.best_schedule = self.schedule.copy() -class TimeSlot(object): - """ - This TimeSlot class is analogous to the TimeSlot class in the models, - i.e. it represents a timeframe in a particular location. - """ - def __init__(self, timeslot_db, verbosity, is_fixed=False): +class GeneratorTimeSlot: + """Representation of a timeslot for the schedule generator""" + def __init__(self, *, verbosity=0, is_fixed=False): """Initialise this object from a TimeSlot model instance.""" self.verbosity = verbosity self.is_fixed = is_fixed + self.overlaps = set() + self.full_overlaps = set() + self.adjacent = set() + self.start = None + self.day = None + self.time_group = 'Unscheduled' + + @property + def is_scheduled(self): + """Will this timeslot appear on a schedule?""" + return self.start is not None + + def store_relations(self, other_timeslots): + pass # no relations + + def has_space_for(self, attendees): + return True # unscheduled time slots can support any number of attendees + + def has_time_for(self, duration): + return True # unscheduled time slots are long enough for any session + + +class DatatrackerTimeSlot(GeneratorTimeSlot): + """TimeSlot on the schedule + + This DatatrackerTimeSlot class is analogous to the TimeSlot class in the models, + i.e. it represents a timeframe in a particular location. + """ + def __init__(self, timeslot_db, **kwargs): + """Initialise this object from a TimeSlot model instance.""" + super().__init__(**kwargs) self.timeslot_pk = timeslot_db.pk self.location_pk = timeslot_db.location.pk self.capacity = timeslot_db.location.capacity @@ -675,33 +730,38 @@ class TimeSlot(object): self.end = self.start + self.duration self.day = calendar.day_name[self.start.weekday()].lower() if self.start.time() < datetime.time(12, 30): - self.time_of_day = 'morning' + time_of_day = 'morning' elif self.start.time() < datetime.time(15, 30): - self.time_of_day = 'afternoon-early' + time_of_day = 'afternoon-early' else: - self.time_of_day = 'afternoon-late' - self.time_group = self.day + '-' + self.time_of_day - self.overlaps = set() - self.full_overlaps = set() - self.adjacent = set() + time_of_day = 'afternoon-late' + self.time_group = f'{self.day}-{time_of_day}' + + def has_space_for(self, attendees): + return self.capacity >= attendees + + def has_time_for(self, duration): + return self.duration >= duration def store_relations(self, other_timeslots): """ Store relations to all other timeslots. This should be called - after all TimeSlot objects have been created. This allows fast - lookups of which TimeSlot objects overlap or are adjacent. + after all DatatrackerTimeSlot objects have been created. This allows fast + lookups of which DatatrackerTimeSlot objects overlap or are adjacent. Note that there is a distinction between an overlap, meaning at least part of the timeslots occur during the same time, and a full overlap, meaning the start and end time are identical. """ for other in other_timeslots: + if other == self or not other.is_scheduled: + continue # no relations with self or unscheduled sessions if any([ self.start < other.start < self.end, self.start < other.end < self.end, self.start >= other.start and self.end <= other.end, - ]) and other != self: + ]): self.overlaps.add(other) - if self.start == other.start and self.end == other.end and other != self: + if self.start == other.start and self.end == other.end: self.full_overlaps.add(other) if ( abs(self.start - other.end) <= datetime.timedelta(minutes=30) or @@ -712,7 +772,7 @@ class TimeSlot(object): class Session(object): """ - This TimeSlot class is analogous to the Session class in the models, + This Session class is analogous to the Session class in the models, i.e. it represents a single session to be scheduled. It also pulls in data about constraints, group parents, etc. """ @@ -810,7 +870,7 @@ class Session(object): ]) def fits_in_timeslot(self, timeslot): - return self.attendees <= timeslot.capacity and self.requested_duration <= timeslot.duration + return timeslot.has_space_for(self.attendees) and timeslot.has_time_for(self.requested_duration) def calculate_cost(self, schedule, my_timeslot, overlapping_sessions, my_sessions, include_fixed=False): """ @@ -820,7 +880,7 @@ class Session(object): The functionality is split into a few methods, to optimise caching. overlapping_sessions is a list of Session objects - my_sessions is an iterable of tuples, each tuple containing a TimeSlot and a Session + my_sessions is an iterable of tuples, each tuple containing a GeneratorTimeSlot and a Session The return value is a tuple of violations (list of strings) and a cost (integer). """ @@ -832,11 +892,11 @@ class Session(object): ) if include_fixed or (not self.is_fixed): - if self.attendees > my_timeslot.capacity: + if not my_timeslot.has_space_for(self.attendees): violations.append('{}: scheduled in too small room'.format(self.group)) cost += self.business_constraint_costs['session_requires_trim'] - if self.requested_duration > my_timeslot.duration: + if not my_timeslot.has_time_for(self.requested_duration): violations.append('{}: scheduled in too short timeslot'.format(self.group)) cost += self.business_constraint_costs['session_requires_trim'] @@ -937,7 +997,7 @@ class Session(object): def _calculate_cost_my_other_sessions(self, my_sessions): """Calculate cost due to other sessions for same group - my_sessions is a set of (TimeSlot, Session) tuples. + my_sessions is a set of (GeneratorTimeSlot, Session) tuples. """ def sort_sessions(timeslot_session_pairs): return sorted(timeslot_session_pairs, key=lambda item: item[1].session_pk) @@ -953,9 +1013,18 @@ class Session(object): cost += self.business_constraint_costs['sessions_out_of_order'] if self.time_relation: - group_days = [t.start.date() for t, s in my_sessions] - # ignore conflict between two fixed sessions - if not (my_sessions[0][1].is_fixed and my_sessions[1][1].is_fixed): + if len(my_sessions) != 2: + # This is not expected to happen. Alert the user if it comes up but proceed - + # the violation scoring may be incorrect but the scheduler won't fail because of this. + warn('"time relation" constraint only makes sense for 2 sessions but {} has {}' + .format(self.group, len(my_sessions))) + + if all(session.is_fixed for _, session in my_sessions): + pass # ignore conflict between fixed sessions + elif not all(timeslot.is_scheduled for timeslot, _ in my_sessions): + pass # ignore constraint if one or both sessions is unscheduled + else: + group_days = [timeslot.start.date() for timeslot, _ in my_sessions] difference_days = abs((group_days[1] - group_days[0]).days) if self.time_relation == 'subsequent-days' and difference_days != 1: violations.append('{}: has time relation subsequent-days but difference is {}' diff --git a/ietf/meeting/models.py b/ietf/meeting/models.py index 603fa1695..068358790 100644 --- a/ietf/meeting/models.py +++ b/ietf/meeting/models.py @@ -567,14 +567,22 @@ class FloorPlan(models.Model): def __str__(self): return u'floorplan-%s-%s' % (self.meeting.number, xslugify(self.name)) + # === Schedules, Sessions, Timeslots and Assignments =========================== +class TimeSlotQuerySet(models.QuerySet): + def that_can_be_scheduled(self): + return self.exclude(type__in=TimeSlot.TYPES_NOT_SCHEDULABLE) + + class TimeSlot(models.Model): """ Everything that would appear on the meeting agenda of a meeting is mapped to a timeslot, including breaks. Sessions are connected to TimeSlots during scheduling. """ + objects = TimeSlotQuerySet.as_manager() + meeting = ForeignKey(Meeting) type = ForeignKey(TimeSlotTypeName) name = models.CharField(max_length=255) @@ -586,6 +594,8 @@ class TimeSlot(models.Model): modified = models.DateTimeField(auto_now=True) # + TYPES_NOT_SCHEDULABLE = ('offagenda', 'reserved', 'unavail') + @property def session(self): if not hasattr(self, "_session_cache"): @@ -1032,11 +1042,16 @@ class SessionQuerySet(models.QuerySet): type__slug='regular' ) + def that_can_be_scheduled(self): + """Queryset containing sessions that should be scheduled for a meeting""" + return self.requests().with_current_status().filter( + current_status__in=['appr', 'schedw', 'scheda', 'sched'] + ) + def requests(self): """Queryset containing sessions that may be handled as requests""" - return self.exclude( - type__in=('offagenda', 'reserved', 'unavail') - ) + return self.exclude(type__in=TimeSlot.TYPES_NOT_SCHEDULABLE) + class Session(models.Model): """Session records that a group should have a session on the diff --git a/ietf/meeting/tests_schedule_generator.py b/ietf/meeting/tests_schedule_generator.py index a88280208..0cc430e18 100644 --- a/ietf/meeting/tests_schedule_generator.py +++ b/ietf/meeting/tests_schedule_generator.py @@ -3,6 +3,8 @@ import calendar import datetime import pytz from io import StringIO +from warnings import filterwarnings + from django.core.management.base import CommandError @@ -107,9 +109,11 @@ class ScheduleGeneratorTest(TestCase): def test_too_many_sessions(self): self._create_basic_sessions() self._create_basic_sessions() - with self.assertRaises(CommandError): - generator = generate_schedule.ScheduleHandler(self.stdout, self.meeting.number, verbosity=0) - generator.run() + filterwarnings('ignore', '"time relation" constraint only makes sense for 2 sessions') + generator = generate_schedule.ScheduleHandler(self.stdout, self.meeting.number, verbosity=1) + generator.run() + self.stdout.seek(0) + self.assertIn('Some sessions will not be scheduled', self.stdout.read()) def test_invalid_meeting_number(self): with self.assertRaises(CommandError): diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index d269bbd70..b90f2506a 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -431,7 +431,6 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None): """ # Need to coordinate this list with types of session requests # that can be created (see, e.g., SessionQuerySet.requests()) - IGNORE_TIMESLOT_TYPES = ('offagenda', 'reserved', 'unavail') meeting = get_meeting(num) if name is None: schedule = meeting.schedule @@ -479,18 +478,18 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None): tombstone_states = ['canceled', 'canceledpa', 'resched'] - sessions = Session.objects.filter(meeting=meeting) + sessions = meeting.session_set.with_current_status() if include_timeslot_types is not None: sessions = sessions.filter(type__in=include_timeslot_types) + sessions_to_schedule = sessions.that_can_be_scheduled() + session_tombstones = sessions.filter( + current_status__in=tombstone_states, pk__in={a.session_id for a in assignments} + ) + sessions = sessions_to_schedule | session_tombstones sessions = add_event_info_to_session_qs( - sessions.exclude( - type__in=IGNORE_TIMESLOT_TYPES, - ).order_by('pk'), + sessions.order_by('pk'), requested_time=True, requested_by=True, - ).filter( - Q(current_status__in=['appr', 'schedw', 'scheda', 'sched']) - | Q(current_status__in=tombstone_states, pk__in={a.session_id for a in assignments}) ).prefetch_related( 'resources', 'group', 'group__parent', 'group__type', 'joint_with_groups', 'purpose', ) @@ -498,9 +497,7 @@ def edit_meeting_schedule(request, num=None, owner=None, name=None): timeslots_qs = TimeSlot.objects.filter(meeting=meeting) if include_timeslot_types is not None: timeslots_qs = timeslots_qs.filter(type__in=include_timeslot_types) - timeslots_qs = timeslots_qs.exclude( - type__in=IGNORE_TIMESLOT_TYPES, - ).prefetch_related('type').order_by('location', 'time', 'name') + timeslots_qs = timeslots_qs.that_can_be_scheduled().prefetch_related('type').order_by('location', 'time', 'name') if timeslots_qs.count() > 0: min_duration = min(t.duration for t in timeslots_qs)