Implement assertions() in search #27 #28
Conversation
There was a problem hiding this comment.
Awesome @jazcap53!
I have some minor comments when checking the optimizer argument. I can already approve it, but I want your opinion as well. You can check that one out 👍
Also, I'd appreciate it if you can rebase your commit similar to the title of your pull request, that is,
Implement assertions() in search (#27)
Normally, what I do is I use the following command when rewording:
$ git rebase -i HEAD~5and when the vim editor starts, I just replace the pick of the specific commit into reword. After that I do a force push
git push origin +masterThank you so much! Just check my suggestions and if you think it's better, then it's all fine! I hope you are enjoying! and thank you again for your contribution!
pyswarms/utils/search/base_search.py
Outdated
| if self.optimizer.__name__ not in ('LocalBestPSO', 'GlobalBestPSO'): | ||
| raise TypeError('Parameter `optimizer` must be of type ' | ||
| 'LocalBestPSO or GlobalBestPSO') | ||
|
|
There was a problem hiding this comment.
This is good for now and I can accept this in its current form. 😄 👍
But I'm wondering how can we scale this when more search methods will be added. For example, Binary PSO is a single-objective method, and I think the same search methods are applicable to it.
The same will go perhaps in multi-objective and constrained optimization. Since the search methods are "independent" whatever optimization method we are using. Meaning, it doesn't really care what kind of problem we are trying to solve, as long as we have an optimize and optimizer supplied as params. Then it's possible to apply the same search methods for these kind of techniques in the future. What do you think @SioKCronin?
I'm thinking of instead checking if the object passed in the optimizer argument belongs to the pyswarms.base module. OR check if the object passed in the optimizer method has an optimize attribute (thus using a hasattr as a checker). What do you think @jazcap53?
There was a problem hiding this comment.
Hi @ljvmiranda921,
I'll of course make the changes you suggest with pleasure. 😄
I'd appreciate it if you can rebase your commit similar to the title of your pull request
I had trouble getting git to accept a commit title with spaces in it. I'll try using \<space> instead of <space>, and if that doesn't work I'll try using %20.
I'm wondering how can we scale this when more search methods will be added
I was wondering the same thing. I had trouble implementing it, though, since the optimizer parameter is a class rather than an instance -- I'll research it further and ask help if I can't find a solution.
I'm enjoying this very much... 👍
There was a problem hiding this comment.
@ljvmiranda921 Figured out that I have to use issubclass(). Simple (embarrassing). 😄
There was a problem hiding this comment.
@ljvmiranda921 o.i.c. if not hasattr(self.optimizer, 'optimize'):
I'll use that unless you would prefer something else 😄
pyswarms/utils/search/base_search.py
Outdated
| @@ -73,7 +90,7 @@ def generate_score(self, options): | |||
| ---------- | |||
|
|
|||
| options: dict | |||
| a dict of 5 hyperparameter values ('c1', 'c2', 'w', 'k', 'p' | |||
| a dict of 5 hyperparameter values ('c1', 'c2', 'w', 'k', 'p' | |||
There was a problem hiding this comment.
Is a parenthesis missing here? 😄 ❓
I guess we can also rephrase this as:
"""
a dict with the following keys: {'c1 ', 'c2', 'w', 'k', 'p'}
"""There was a problem hiding this comment.
Is a parenthesis missing here?
Sure enough. 😄 I was afraid to change that line, though -- it's not mine and I don't know what the etiquette is in such a case. It just appears in the diff because my vim is set to eliminate trailing spaces on save of a .py file. I'll rephrase it according to your suggestion.
Check that optimizer member in SearchBase has an optimize attribute. Check type of n_selection_iters member in RandomSearch. Add self.assertions() line to __init__() in each of Searchbase, GridSearch, and RandomSearch. Add call to super().assertions() in RandomSearch. Author: jazcap53 Email: andrew.jarcho@gmail.com
|
Hi @ljvmiranda921 ! I've followed all instructions, I hope correctly. Google and Stack Overflow have been getting a lot of my business lately! 😄 If there's anything else I need to do, please of course let me know. Next I plan to study up on Travis CI and write some tests for the 'Go the extra mile?' portion of the Issue. 👍 |
|
Hi @jazcap53 ! All is good! I will merge this now 👍 Congratulations for your first contribution!!! :D For the extra mile, no need to actually study travis-ci because I have already set that one up beforehand. You just need to write the unit tests, and travis-ci will automatically run your tests using So in case that one of the tests didn't pass, it will send a notification here in Github and also in my e-mail. In the 'Extra mile', you will just write the unit tests that travis-ci will run. 😄 |
|
@ljvmiranda921 |
… (#27)
Check name of optimizer member in SearchBase.
Check type of n_selection_iters member in RandomSearch.
Add self.assertions() line to init() in each of Searchbase, GridSearch, and RandomSearch.
Add call to super().assertions() in RandomSearch.
Author: jazcap53
Email: andrew.jarcho@gmail.com