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.

Sign up for GitHub

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

Jump to bottom

WIP: Move CLUE algo in external library #18

Closed
wants to merge 5 commits into from
Closed

WIP: Move CLUE algo in external library #18

wants to merge 5 commits into from

Conversation

ebrondol
Copy link
Collaborator

@ebrondol ebrondol commented Feb 3, 2022 •
edited
Loading

BEGINRELEASENOTES

  • The CLUE algorithm (both CPU and GPU implementations) are now an external library
  • Currently the library was created with the files included in this package
  • In the future the library will be created and updated using the standalone repository

ENDRELEASENOTES

Fix #16

@ebrondol ebrondol requested review from vvolkl and fdplacido February 3, 2022 17:11
@ebrondol
Copy link
Collaborator Author

ebrondol commented Feb 3, 2022 •
edited
Loading

I have tried to follow the suggestions included in #17
Also this PR is very much open to discussion and further suggestions from @fdplacido and @vvolkl

andresailer
andresailer reviewed Feb 4, 2022
View reviewed changes
Copy link
Contributor

@andresailer andresailer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some things could be simplified.

src/clue/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue_cuda_cupla/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue_cuda_cupla/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue_cuda_cupla/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue_tbb_cupla/CMakeLists.txt Outdated Show resolved Hide resolved
src/clue_tbb_cupla/CMakeLists.txt Show resolved Hide resolved
src/clue/CMakeLists.txt Outdated Show resolved Hide resolved
fdplacido
fdplacido approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@fdplacido fdplacido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I overall agree with Andre's comments, and looks good to me now.

@ebrondol
Copy link
Collaborator Author

I should have implemented all the comments from @andresailer.
If there are no more comments, I will close this PR and prepare everything to be ready on the main repository side. Thanks again for the help.

@andresailer
Copy link
Contributor

Looks good to me, thanks!

@ebrondol ebrondol closed this Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andresailer andresailer andresailer left review comments

@fdplacido fdplacido fdplacido approved these changes

@vvolkl vvolkl Awaiting requested review from vvolkl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move CLUE algo in external library
3 participants
@ebrondol @andresailer @fdplacido