[ENH] Implement Proximity Forest classifier #1729
Conversation
aeon-actions-bot
bot
added
classification
Thank you for contributing to
|
itsdivya1309
requested review from
MatthewMiddlehurst and
TonyBagnall
as code owners
|
This needs to be included in the API documentation also. |
| def _fit_tree(self, X, y): | ||
| clf = ProximityTree( | ||
| n_splitters=self.n_splitters, | ||
| max_depth=self.max_depth, | ||
| min_samples_split=self.min_samples_split, | ||
| random_state=self.random_state, | ||
| n_jobs=self.n_jobs, | ||
| ) | ||
| clf.fit(X, y) | ||
| return clf |
There was a problem hiding this comment.
Similar comment for predict, but I think it might be better to define the function you parallelize with joblib outside of the object you call them from. Something to do with the fact that joblib pickling the objects you parallelize, if I remember right ? This might mean that you create a copy of the ProximityForest object every time you call _fit_tree .
To avoid that, you would define _fit_tree as a function outside ProximityForest.
There was a problem hiding this comment.
Thanks for pointing this out.
There was a problem hiding this comment.
Is this true? I think we have functions elsewhere that do this. Interesting to see if that needs to be changed.
aeon/classification/distance_based/tests/test_proximity_forest.py
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
| def _predict_proba(self, X): | ||
| output_probas = Parallel(n_jobs=self._n_jobs, prefer="threads")( |
There was a problem hiding this comment.
We discussed the need of a parameter for the joblib backend (i.e. threads vs processes), you should add a class parameter that default to threads.
It might be better to use the backend parameter instead of the prefer (see docs) to have a more fine grained control over the chosen backend.
Reference Issues/PRs
Closes #159
What does this implement/fix? Explain your changes.
Implementation of Proximity Forest Algorithm using the Proximity Trees.