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

Fix relative include in mpi.hpp #120

Open
cwpearson opened this issue Sep 9, 2024 · 3 comments · May be fixed by #127
Open

Fix relative include in mpi.hpp #120

cwpearson opened this issue Sep 9, 2024 · 3 comments · May be fixed by #127

Comments

@cwpearson
Copy link
Collaborator

#include "../concepts.hpp"

@dssgabriel
Copy link
Collaborator

PR #122 should fix this. If we merge it, I have another PR ready that updates all the includes with the <KokkosComm/...> syntax. I preferred splitting them so not to touch every file at the same time as these are pretty big changes.

Additionally, @cedricchevalier19 mentioned that having relative path includes may improve compilation speed.

@nicoleavans
Copy link
Collaborator

PR #122 was merged; how best to proceed? <KokkosComm/...> syntax seems like a good solution.

@dssgabriel
Copy link
Collaborator

dssgabriel commented Nov 4, 2024

I can make a quick PR replacing all KokkosComm-related includes with the <KokkosComm/...> system-style includes (I may even have a working branch with this change at the ready).

However, in a discussion with @cedricchevalier19, he mentioned that these kinds of includes may have a negative impact on compile times (see previous message in this thread). I didn't notice any slowdowns for KokkosComm builds but we may want to take that into consideration before changing all the paths.
Any suggestions before we proceed?

dssgabriel added a commit to dssgabriel/kokkos-comm that referenced this issue Nov 4, 2024
Removes all instances of relative paths in favor of system include-style
syntax: `<KokkosComm/...>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants