fix unbound activities planned during bound activities

program_code/algorithms/randomize.py

@@ -91,7 +91,7 @@ def assign_activities_timeslots_once(self, schedule: Schedule):
         random.shuffle(activities)
         random.shuffle(timeslots)
 
-        self.assign_activities_timeslots_sorted(schedule, activities, timeslots)
+        self.assign_activities_timeslots_prioritized(schedule, activities, timeslots)
 
     def assign_activities_timeslots_uniform(self, schedule: Schedule):
         # Make shuffled list of timeslots so they will be picked randomly

program_code/algorithms/solver.py

@@ -39,38 +39,34 @@ def assign_activities_timeslots_greedy(
                     # Reached required capacity, stop looking for timeslots
                     break
 
-                # Check if timeslot can be coupled to activity (max_timeslots, timeslot available)
+                # Check if timeslot can be coupled to activity
+                # (max_timeslots, timeslot available, moment is already taken by same course lecture)
                 if not self.can_assign_timeslot_activity(timeslot, activity):
                     continue
 
-                # Check if moment is already taken by a course lecture
-                for bound_activity in activity.course.bound_activities.values():
-                    if self.node_has_period(bound_activity, timeslot):
-                        continue
                 schedule.connect_nodes(activity, timeslot)
 
                 if not activity.capacity:
                     added_capacity = timeslot.capacity
                 else:
                     added_capacity = min(activity.capacity, timeslot.capacity)
+
                 total_capacity += added_capacity
 
             if self.verbose and total_capacity < activity_enrolments:
                 warnings.warn(f"FAILED: {total_capacity, activity.enrolled_students}")
 
-    def assign_activities_timeslots_sorted(self, schedule, activities, timeslots):
+    def assign_activities_timeslots_prioritized(self, schedule: Schedule, activities, timeslots):
         # Activities with max timeslots
         activities_bound: list[Activity] = []
         # Activitities with unbound number of timeslots
         activities_free: list[Activity] = []
 
         # Sort activities into bound and unbound max_timeslots
         # TODO: these lines are degenerate of course.bound_activities
-        for activity in activities:
-            if activity.max_timeslots:
-                activities_bound.append(activity)
-            else:
-                activities_free.append(activity)
+        for course in schedule.courses.values():
+            activities_bound.extend(course.bound_activities.values())
+            activities_free.extend(course.unbound_activities.values())
 
         # first assign activities with max timeslots most efficiently
         self.assign_activities_timeslots_greedy(schedule, activities_bound, timeslots, reverse=True)

program_code/algorithms/statistics.py

@@ -54,16 +54,19 @@ def node_has_period(self, node, timeslot: Timeslot):
                 return True
         return False
 
-    def moment_conflicts(self, activities1, activities2):
+    def moment_conflicts(self, nodes1, nodes2):
+        """Count same time bookings between `nodes1` and `nodes2`"""
         conflicts = 0
-        for activity1 in activities1:
-            for activity2 in activities2:
-                if activity1.id == activity2.id:
+        for node1 in nodes1:
+            for node2 in nodes2:
+                if node1.id == node2.id:
                     continue
-                for timeslot1 in activity1.timeslots.values():
-                    for timeslot2 in activity2.timeslots.values():
+                for timeslot1 in node1.timeslots.values():
+                    for timeslot2 in node2.timeslots.values():
                         if timeslot1.moment == timeslot2.moment:
                             conflicts += 1
+        # TODO: possible this has to be divided by two when nodes1 == nodes2
+        # TODO: or build index on pairs checked
         return conflicts
 
     def student_has_activity_assigned(self, student: Student, activity: Activity):
@@ -108,6 +111,11 @@ def can_assign_timeslot_activity(self, timeslot: Timeslot, activity: Activity):
         if self.node_has_activity(timeslot):
             return False
 
+        # Check whether timeslot is already taken by a lecture of the same course
+        for bound_activity in activity.course.bound_activities.values():
+            if self.node_has_period(bound_activity, timeslot):
+                return False
+
         # Can always assign timeslots to tutorials/practicals
         if activity.max_timeslots is None:
             return True
@@ -119,6 +127,7 @@ def can_assign_timeslot_activity(self, timeslot: Timeslot, activity: Activity):
         # One instance of activity means all students must fit in room
         if activity.max_timeslots == 1 and timeslot.room.capacity >= activity.enrolled_students:
             return True
+
         return len(activity.timeslots) < activity.max_timeslots
 
     def can_assign_student_timeslot(self, student: Student, timeslot: Timeslot):

program_code/classes/result.py

@@ -36,10 +36,11 @@ def check_solved(self):
         assert self._compressed == False, "Can only verify schedule uncompressed."
         # TODO check if it is solved (meets all hard constraints). Use the above hard constraint checkers.
         for course in self.schedule.courses.values():
+            # Check whether lectures never coincide with same course activities
+            if self.moment_conflicts(course.bound_activities.values(), course.activities.values()):
+                return False
+
             for activity in course.activities.values():
-                # Check whether lectures never coincide with same course activities
-                if self.moment_conflicts(course.bound_activities.values(), course.activities.values()):
-                    return False
                 # Check lectures have one timeslot
                 if self.activity_overbooked(activity):
                     return False