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

TaskGenerator should allow user controlled UUIDs for their Tasks #24

Closed
PetrosPapapa opened this issue Sep 3, 2020 · 9 comments · Fixed by #25
Closed

TaskGenerator should allow user controlled UUIDs for their Tasks #24

PetrosPapapa opened this issue Sep 3, 2020 · 9 comments · Fixed by #25
Assignees
Labels
feature New feature or request

Comments

@PetrosPapapa
Copy link
Member

No description provided.

@MBaczun
Copy link
Contributor

MBaczun commented Sep 6, 2020

I think this is now implemented.
The next question is, why are resources not considered part of a task, and in turn part of a taskGenerator? Maybe this could be combined in a similar way?

@PetrosPapapa
Copy link
Member Author

How do you mean? The actual TaskResource objects? Because they are shared among all tasks. They are also mutable (at least in the current implementation).

I am also considering more flexible resource matching (#5) than just directly through names. This, however, will make the Scheduler much more complicated, so I've been holding off from trying it.

@MBaczun
Copy link
Contributor

MBaczun commented Sep 7, 2020

Currently whenever I pass a task generator I also need to pass a Seq[String] of task resources. It might be nice if the task generator contained this Seq instead of always passing it separately. Maybe it's wrong, but I think of the resources that a task uses to be an attribute/property that defines this task, so I always found it strange that it's not treated like a part of the task. I don't mean that resources belong exclusively to any single task, but that the resources that are required for a task are, in a way, what defines the task.

@PetrosPapapa
Copy link
Member Author

Ah I see what you mean now. I can't currently think of a reason not to add this too to the generator, so go ahead!

@MBaczun
Copy link
Contributor

MBaczun commented Sep 7, 2020

ok! it's added on the same branch.

@PetrosPapapa
Copy link
Member Author

Good stuff!

@PetrosPapapa
Copy link
Member Author

Do we actually need a TaskGenerator at all now? It can no longer generate more than one task anyway (it used to, but not any more). The only thing it does is pick random values. Maybe it's just better to pass Tasks to the Coordinator? One can still use generators externally if they want.

@MBaczun
Copy link
Contributor

MBaczun commented Sep 8, 2020

That's a good point. In some ways it would simplify the code. However, there is a benefit to TaskGenerator.create(), which is that the person calling .create can specify the time at which a task is created. When the coordinator does this, it has easy access to the current time, unlike the simulation which has no access to the current time.
I use the creation time of a task in lookahead too, and this creation time is set by calling .create in the scheduler (which has access to both the current time and the lookahead function which describes the starting time of a task given its predecessors).

@PetrosPapapa
Copy link
Member Author

Ah yes that's a good point, but I think that can also be circumvented in a few ways... Simplifying the code would be more useful.

Just leave it as is for now, and I'll start a new issue to consider this in the future. We ought to keep the focus on the lookahead stuff right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants