search

TimFinucane / Ainur

Solution for parallel task scheduling problem, inspired by the tale of the Lord of the Rings
Other
5 stars 0 forks source link

Bugs/algorithm #85

Closed Buster-Darragh-Major closed 6 years ago

Buster-Darragh-Major commented 6 years ago

Resolves #80, Resolves #81

Fixes Bugs found in DFSAlgorithm and semantics with lower bound. Lower bound now gets the lower bound of a full schedule produce-able by that partial schedule opposed to getting the lower bound of cost increase. i.e. it now takes into account the input partial schedules size.

Relative speed improvements?

Fixed single test in CriticalPathTests - changed assumption that answer should include full schedule.

TimFinucane commented 6 years ago

Where is this issue present in code? And what do you propose the change is, i'm not quite sure i follow what the problem is here

nathan-cairns commented 6 years ago

For example in dfs algorithm we have

int[] earliestStarts = calculateEarliestTimes(graph, curSchedule, node);

Whereas the context should be made clear as follows

int[] earliestStarts = Helper.calculateEarliestTimes(graph, curSchedule, node);

Also I feel like helper could have a more descriptive name. Like AlgorithmUtils

So ideally we'd have something like

int[] earliestStarts = AlgorithmUtils.calculateEarliestTimes(graph, curSchedule, node);
nathan-cairns commented 6 years ago

Oops accidentally closed sorry am on mobile

TimFinucane commented 6 years ago

That sounds fine by me. This PR, despite the accidental use of a helpers import, has nothing to do with the helpers class so if you want you could make a separate pull request with that change, but i dont think its appropriate for this PR

maddiebeagley commented 6 years ago

I've actually already included these changes in the PR i'm currently working on

nathan-cairns commented 6 years ago

Easy wew