Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CUDARelativeDifferencePrior #1408
CUDARelativeDifferencePrior #1408
Changes from 7 commits
5745b8c
2c4fa32
52c4943
f41728e
d3f3732
063633e
483cc13
7e26bd4
0249e93
843cd13
5eec80b
2e90b17
13def4f
5ddce33
1a7d4d4
1c61543
5b3f768
8cc358f
08d0f81
227c48e
95e8428
aed4a1d
f163578
f79f95b
35937dd
9e8daae
2e45fdc
304f121
8825485
50ae299
51ef4fc
06ec3de
3e04184
a5ccc4c
d2f6d6d
c31953e
a0114ab
ef66303
c634a0d
165da28
7ac03fd
fa4d7fe
ccc962a
dcd3b86
3e43ca7
66c082b
7a2cf76
44dad5a
d4f2c9b
05cdfaf
1f0d692
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I really hope we don't need
CUDA_INCLUDE_DIRS
for the registries. STIR .h files ideally have no CUDA includes at all. It's going to create trouble.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.
I thought we'd want the target to be the registeries as we need the cuda_runtime.h when building cuda files that will be in the registry... Also I couldn't quite figure out where else to put the target_include_directories, where would you suggest?
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.
As far as I can see,
cuda_runtime.h should NOT be included in src/include/stir/recon_buildblock/CUDA/CudaRelativeDifferencePrior.h, but in the .cxx/.cu. That means that the registries wouldn't need to know about the CUDA include dir, therefore you don't need the
target_include_directories` at all. (The .cu files need it, but they get it by depending on CUDA::cudart target).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.
move to cxx
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.
this is done but dim3 is cuda specific, I changed it for a native struct and removed the cuda_runtime include, aren't the other two needed?