Added parallel search logic #10
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fedetask
approved these changes
Oct 13, 2021
There was a problem hiding this comment.
Nice work! I don't spot any obvious problem at this moment.
One question about the stile: in SearchJobInstance you used the type hints for the return value like def fun(args) -> float:, do you think it would make sense to consistently use it in all functions? (Along with argument type hints)
Other than that, I like the idea of letting the user define the job class.
CarolinaBianchi
approved these changes
Oct 16, 2021
Comment on lines
+34
to
+44
| @abstractmethod | ||
| def kill(self) -> None: | ||
| """Finish job | ||
| """ | ||
| raise NotImplementedError | ||
|
|
||
| @abstractmethod | ||
| def end(self) -> None: | ||
| """Run any task necessary when done | ||
| """ | ||
| raise NotImplementedError |
There was a problem hiding this comment.
Are these two functions called by anyone? If not, I would not include them in the abstract SearchJobInstance
There was a problem hiding this comment.
They are not called now. I thought they might be useful but you are right, they are not needed.
I'll remove them for now :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey guys (@fedetask @CarolinaBianchi @cserranocolome ), so the code is pure garbage but I use it a lot and I wanted to have parallel optimization
So in an effort to make it a bit cleaner I refactored it a bit and added a ParallelSearcher class which schedules calls to SearchJobInstance abstract class.
This SearchJobInstance has to be implemented by the user populating functions to launch a particular job, check if it is finished, get the result and this sort of stuff
Let me know if you have any comments on this (or if you care at all) otherwise I'll just merge it to master. One day it would be nice to make a major cleanup of this repo though