Skip to content
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

[WIP] Further optimizations to sparse semiring primitives #1

Closed
wants to merge 3 commits into from

Conversation

cjnolet
Copy link
Owner

@cjnolet cjnolet commented Feb 5, 2021

No description provided.

lowener and others added 3 commits February 5, 2021 04:08
Closes #2864.

This PR will replace `datasets.make_blobs` and `metrics.accuracy_score` from sklearn to cuml.

Authors:
  - Micka (@lowener)

Approvers:
  - John Zedlewski (@JohnZed)

URL: #3408
Closes #3376

This PR moves a few header files around to fix bad dependencies from `include` -> `src`. Moving forward, the only allowed `#include` direction will be:

- `src` -> `include`
- `src` -> `src_prims`
- `src_prims` -> `include`

To facilitate this, a few changes needed to be made:
1. Functions for the C-API have been separated into their own `*_api.cpp` file (some were combined with C++ files)
2. `host_buffer`, `device_buffer`, `hostAllocator` and `deviceAllocator` were moved from `src_prims` to `include`
3. `#include <common/cumlHandle.hpp>` has been removed from all C++ files.

This PR includes some additional improvements:
- Updated the `include_checker.py` script:
   - Added functionality to check for badly placed `#pragma once`
   - Added functionality to fix some of the existing warnings
   - General refactor to support more checks
- Fixed all incorrect uses of `#pragma once`
- Fixed incorrect uses of `using namespace ...` in header files outside of a namespace
- Removed some unnecessary `#include`

While this touches a lot of files, the actual number of changes is relatively small. Below is a before/after comparison of the include graphs:

**`src/include`:**
Before:
![image](https://user-images.githubusercontent.com/42954918/105561661-ab32fe00-5cd4-11eb-8850-bfeaffaba60f.png)
After:
![image](https://user-images.githubusercontent.com/42954918/105561673-b4bc6600-5cd4-11eb-8bb7-04be91f902b6.png)

**`src/src_prims`:**
Before:
![image](https://user-images.githubusercontent.com/42954918/105561616-79ba3280-5cd4-11eb-8a49-1d0c030df7c1.png)
After:
![image](https://user-images.githubusercontent.com/42954918/105561633-89397b80-5cd4-11eb-9702-27b2a809834b.png)

Authors:
  - Michael Demoret (@mdemoret-nv)

Approvers:
  - Dante Gama Dessavre (@dantegd)
  - John Zedlewski (@JohnZed)
  - Thejaswi. N. S (@teju85)

URL: #3402
This solve partially #3435 by fixing the documentation of `SimpleImputer`. The next step will be to allow usage of lists for this algorithm.

Authors:
  - Micka (@lowener)

Approvers:
  - Michael Demoret (@mdemoret-nv)

URL: #3447
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants