Ensemble builder #4
Conversation
| ) -> "AbstractEnsemble": | ||
| return self | ||
|
|
||
| def get_identifiers_from_run_history(self) -> List[Tuple[int, int, float]]: |
There was a problem hiding this comment.
We can write the code more concisely as follows:
min_idx = np.array([run_value.cost for run_value in self.run_history.data.values()]).argmin()
min_key = list(self.run_history.data.keys())[min_idx]
min_run_value = self.run_history.data[min_key]
if min_run_value < best_model_loss:
...
else: # SMAC did not work properly
raise ValueError
There was a problem hiding this comment.
But there are plenty of this to check, like if the path exists (it might have been deleted by ensemble builder), among others. This suggestion does not align to that. Yes, one could do list comprehension with filtering, but then it is not much different than the actual code...
There was a problem hiding this comment.
I will check here later (my time schedule is temporally tight..., I think I can check on this weekend)
| self.backend = backend | ||
|
|
||
| # Add some default values -- at least 1 model in ensemble is assumed | ||
| self.indices_ = [0] |
There was a problem hiding this comment.
Why indices_, but not _indices?
foo_ is for reserved words such as class, list.
_foo is for private variables.
There was a problem hiding this comment.
But this is a private variable?
There was a problem hiding this comment.
I do know the usage of indices, so I'm partially asking.
If you use indices_, you should specify the intentions.
| ) | ||
| s = len(ensemble) | ||
| if s > 0: | ||
| np.add( |
There was a problem hiding this comment.
This part consumes the same amount as the following:
weighted_ensemble_prediction += ensemble[-1]
It is shorter and understandable.
| ) | ||
| s = len(ensemble) | ||
| if s > 0: | ||
| np.add( |
There was a problem hiding this comment.
This part consumes the same amount as the following:
weighted_ensemble_prediction += ensemble[-1]
This is a proof of concept PR. It is fully functional, yet it assumes we cannot merge the metrics from Auto-Sklearn and Auto-Pytorch.
Today they differ slightly and for those reasons, I have proposed this or to take in a loss function when doing ensemble selection.
It also removed many lines that are legacy.