-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HW_18 #4
base: main
Are you sure you want to change the base?
HW_18 #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Параллельный лес: 22/25
- Не везде есть docstring-и и аннотации к типам -4б
bioinfUtils.py
— неподходящее название (camelCase , когда все остальные файлы snake_case нехорошо ) — лучше такbioinf_utils.py
-1б- 3/4 примера в jupyter-ноутбуке
Showcases.ipynb
-5б - кажется, вы забыли добавить
@pytest.fixture
к входным параметрам тестов, без этого они не запустятся
(только, если не вручную запускать — но тогда пропадает смысл использованияpytest
). -3б
+1 балл за красивый и информативный readme
Итого: 35 баллов
|
||
self.classes_ = sorted(np.unique(y)) | ||
|
||
with ThreadPoolExecutor(n_jobs) as pool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-3 балла (CPU-bound задача - нужны процессы)
То есть нужно было использовать ProcessPoolExecutor
tree = DecisionTreeClassifier( | ||
max_depth=self.max_depth, max_features=self.max_features, random_state=self.random_state+i) | ||
|
||
temp_pool.append(pool.submit(tree.fit, X_sample, y_sample)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
непосредственный фитинг деревьев вы сделали параллельным (псевдо, конечно)
но забыли про подготовку данных и создание объекта дерева -- было бы здорово также делать это параллельно. :)
temp_pool = [pool.submit(tree.predict_proba, X[:, self.feat_ids_by_tree[i]]) | ||
for i, tree in enumerate(self.trees)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в целом ок, но почему не map
?)
No description provided.