Some issues with branch "visualize_heap"
Hi @alsultanm, I use this issue to collect some comments I have on the code you pushed this week.
-
here it should be
Matrix.zeros
instead ofMatrix.ones
- uncomment this line
- check spelling here
- I have been thinking about the class
Tasks
defined here. The problem here is that tasks alone are not able to represent their order for producing one job; therefore, the initialization of a JobShop feels a bit clunky, as the user needs to provide the tasks in order with respect to the jobs. Moreover, this forces us to import a new library,collections
, which makes the code more complicated to use and understand. Probably I would modify this as follows. Instead of defining a class for tasks, I would define one for jobs as follows.
class Job:
def __init__(self, name: str, duration: list[float], machine: list[str])-> None:
if len(durations) != len(machines):
# throw error
self.name = name
self.durations = durations
self.machines = machines
When creating an object of class Job
, the user is forced to provide the tasks for a specific job in the right order. This will simplify the initialization of a Jobshop
object. If you need to give a (string) name to tasks, just append to the name of the job the number representing the position of the task in the order for producing one job (as in the paper). For instance, if we have a job with name a
, its tasks should be automatically called a1
, a2
, a3
, and so on.
- Since most of the code in
computeThroughput()
(here) works on max-plus automata, it would make sense to write a method of classAutomaton
with namecomputeThroughput()
, and then simply call it inJobshop.computeThroughput()
as follows:
def computeThroughput(self, schedule: [str])-> float:
"""
Compute the throughput of a schedule.
Attributes
----------
Jobshop : Jobshop
The jobshop to be analyzed.
schedule : list
The schedule to be analyzed.
Returns
-------
float
The throughput of the schedule.
"""
heap = self.jobshop2Heap()
mpa = heap.heap2MPA()
return mpa.computeThroughput(schedule)
If you notice, in my code there is no input jobName
. Indeed, we do not need it. The string schedule
should indicate the periodic sequence of tasks to be analyzed, and that's it, nothing else is needed. If methods jobshop2Heap()
and heap2MPA()
are written correctly, each task in the schedule should correspond to an event of the max-plus automaton.
- In general the code is very easy to read and well commented. Well done!