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

[ENH] [FINAL] Header structure: combine all PRs into one #1469

Merged

Conversation

ahendriksen
Copy link
Contributor

@ahendriksen ahendriksen commented Apr 27, 2023

@ahendriksen ahendriksen requested review from a team as code owners April 27, 2023 11:07
@ahendriksen ahendriksen self-assigned this Apr 27, 2023
@ahendriksen ahendriksen added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Build Time Improvement labels Apr 27, 2023
@ahendriksen ahendriksen force-pushed the enh-header-structure-final branch from 777d944 to 0b3946a Compare April 27, 2023 12:50
@ahendriksen ahendriksen requested review from a team as code owners April 27, 2023 12:50
@divyegala
Copy link
Member

Is the rebase done correctly? CI is saying it's still 14 commits behind

ahendriksen and others added 12 commits April 27, 2023 15:12
Under multiple combinations of RAFT_EXPLICIT_INSTANTIATE_ONLY and RAFT_COMPILED
Co-authored-by: Divye Gala <divyegala@gmail.com>
The compute_similarity and interleaved_scan kernel are quite expensive
to compile. Splitting the headers in this commit.
These two include files are likely to transitively include spdlog, which
increases compilation times.
@ahendriksen ahendriksen force-pushed the enh-header-structure-final branch from 0b3946a to fd8386c Compare April 27, 2023 13:14
@ahendriksen
Copy link
Contributor Author

ahendriksen commented Apr 27, 2023

Is the rebase done correctly? CI is saying it's still 14 commits behind

I missed a ifndef in a previous version of commit 7af7711. Then I redid the rebase and naively thought it would be alright (which it wasn't). Now I have done the rebase from the beginning again. All commits should now be part of this PR.

@ajschmidt8 ajschmidt8 removed the request for review from a team April 27, 2023 13:29
@cjnolet
Copy link
Member

cjnolet commented Apr 28, 2023

/merge

@rapids-bot rapids-bot bot merged commit fbce1a4 into rapidsai:branch-23.06 Apr 28, 2023
rapids-bot bot pushed a commit that referenced this pull request May 4, 2023
The `format` function is used by [debug and trace loggers](https://github.com/rapidsai/raft/blob/a44ca96c5cddec7ca67b510f3c21163d3958bc7e/cpp/include/raft/core/logger-macros.hpp#L44-L75). While PR #1469 has restructured the logger headers it was forgotten to expose `detail::format` in case the `RAFT_EXPLICIT_INSTANTIATE_ONLY` is defined. This PR fixes that.

Authors:
  - Tamas Bela Feher (https://github.com/tfeher)
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Allard Hendriksen (https://github.com/ahendriksen)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1482
rapids-bot bot pushed a commit that referenced this pull request May 4, 2023
The return type of `get_pool_memory_resource` was changed in #1469 from `pool_memory_resource` to `device_memory_resource`. There are debug logs in the code ([example](https://github.com/rapidsai/raft/blob/a44ca96c5cddec7ca67b510f3c21163d3958bc7e/cpp/include/raft/neighbors/detail/ivf_pq_build.cuh#L1328-L1329)), which query the pool size, that would fail when debug logging is enabled. This PR removes the `pool_size() ` calls, so that the code can be compiled with debug mode on.

Authors:
  - Tamas Bela Feher (https://github.com/tfeher)
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Time Improvement ci CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
Development

Successfully merging this pull request may close these issues.

[DISCUSSION] A robust solution to precompiling function templates
3 participants