Make all algorithms in cuda/std/algorithm available through cuda/std/algorithm.meow.h#7935
Make all algorithms in cuda/std/algorithm available through cuda/std/algorithm.meow.h#7935miscco wants to merge 1 commit intoNVIDIA:mainfrom
cuda/std/algorithm available through cuda/std/algorithm.meow.h#7935Conversation
|
I am proposing to keep the current folder structure intact and For me this greatly improves maintainability of our code base. Otherwise there are 500+ files in |
cabe671 to
d7051d6
Compare
This comment has been minimized.
This comment has been minimized.
d7051d6 to
b0a1755
Compare
This comment has been minimized.
This comment has been minimized.
c8dd140 to
36bc8b6
Compare
This comment has been minimized.
This comment has been minimized.
I don't think symlinks work on Windows, do they? |
|
Given this is a pretty extensive product facing change, I'd like more thoughts from @wmaxey @griwes @gevtushenko @brycelelbach @ericniebler |
They dont, moved to forwarding headers |
This comment has been minimized.
This comment has been minimized.
jrhemstad
left a comment
There was a problem hiding this comment.
Some questions I'd like answered before we merge this:
- How will we test this? I think we should have tests that include each micro-header and ensure that was is supposed to be available in that header is indeed available.
- How will we document this?
- I'd like to see an evaluation of how well this naming pattern will extend to other headers
That is exactly what I meant. The chosen character does not appear in any source file name in the repository outside of the file extension currently we only have As long as there is never a feature that we want to publicize named "h" and "cuh" we are fine. The patter works perfectly fine with |
|
I agree with @miscco here that
That is still valuable to have independently. And it may give us new insight after all. @miscco can we gather a list of headers that would benefit from splitting as well? Like |
I believe the candidates are:
|
Can we have some really minimal tests for that? |
cbad07a to
53408de
Compare
This comment has been minimized.
This comment has been minimized.
53408de to
ffb2378
Compare
ffb2378 to
6439853
Compare
This comment has been minimized.
This comment has been minimized.
|
@jrhemstad I have added tests to verify everything work with just the modularized includes, and it even found a bug 🎉 Any other questions? |
6439853 to
6df918f
Compare
This comment has been minimized.
This comment has been minimized.
|
i really do not like the naming scheme for the files. tooling very much cares about extensions. i don't expect this to be a great experience in an IDE, for example. and if we modularize all the headers listed in this comment from @Missco, then we'll have 100s of headers in
then why aren't we using that? |
The issue is that it is surprising and non-trivial. Note that we cannot do So for every header we need to find a similar but not equal name for the original header which is really awkward. It also breaks down with e.g. I would also like to note that we have a bazillion files in our CI that are named |
so what? we can find the names. the cure you're suggesting is worse than the disease.
|
I disagree. I think discoverability of the single function headers is also very relevant for IDE usage, so if the user types Can't we just take the umbrella header and append a single character for the directory name? Like |
brycelelbach
left a comment
There was a problem hiding this comment.
I think we must do meow.suffix not algorithm.meow.
- Not having a suffix messes up syntax highlighting which is painful for users.
- It's probably also not great for AI agents. A suffix tells the AI agent what the file is without looking at it.
We should ideally use just one suffix for header names; let's go with whatever is most consistent with our current best practices. The options are .h, .hpp, .cuh; my understanding is that .cuh is considered best practice and what we try to use.
…td/algorithm.meow`
6df918f to
5294ffa
Compare
3d9c513 to
5294ffa
Compare
🥳 CI Workflow Results🟩 Finished in 1h 44m: Pass: 100%/108 | Total: 2d 06h | Max: 1h 23m | Hits: 83%/296842See results here. |
cuda/std/algorithm available through cuda/std/algorithm.meowcuda/std/algorithm available through cuda/std/algorithm.meow.h
We had a poll to see how users would like to access individual features of umbrella headers like
<cuda/std/algorithm>The vast majority preferred a pattern like
<cuda/std/algorithms/find.h>However, we can get away with finding a similar but different name if we use the pattern
umbrella_header.featureWe can create a wrapper for the internal file and also instruct our IDE to ignore all files that match the pattern
[a-z_]*\.[a-z_]*This works because we do not have any.hfiles in neither<cuda/>not<cuda/std/>