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

Conversion to new ETI requiring minimal C++ code additions #496

Closed
wants to merge 4 commits into from

Conversation

jjwilke
Copy link
Contributor

@jjwilke jjwilke commented Nov 15, 2019

No description provided.

@jjwilke jjwilke changed the base branch from master to develop November 15, 2019 00:49
@jjwilke
Copy link
Contributor Author

jjwilke commented Nov 15, 2019

Work in progress. PR open if anyone wants to review the initial implementation and give feedback before renovating the entire repository.

Copy link
Contributor

@mhoemmen mhoemmen left a comment

Choose a reason for hiding this comment

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

It's great to get rid of the files we don't need that nevertheless occupy build time :-) . Will the next step be removing redundant instantiations (e.g., Cuda, AnonymousSpace replacing the double instantiations for CudaUVMSpace and CudaSpace)?

@jjwilke
Copy link
Contributor Author

jjwilke commented Nov 16, 2019

@mhoemmen I think changing to AnonymousSpace is above my pay grade. My understanding is that CudaSpace is "zero-overhead" convertible to AnonymousSpace, so this probably makes sense to do? I'm not familiar enough with the code base to know how much algorithm specialization based on the memory space? I know we do specialization based on the execution space. @kyungjoo-kim @srajama1

@jjwilke jjwilke removed the InDevelop label Nov 16, 2019
@mhoemmen
Copy link
Contributor

mhoemmen commented Nov 18, 2019

@jjwilke We've talked about this before; I just want people to keep it in mind that kokkos-kernels likely has redundant instantiations that consume build time and space. Functions that don't allocate Views really don't need to have multiple instantiations for different memory spaces but the same execution space.

@jjwilke
Copy link
Contributor Author

jjwilke commented Dec 13, 2019

Too many changes including file renames for this PR to remain useful. Canceled in favor of PR #526. Cherry-picked commits into new PR.

@jjwilke jjwilke closed this Dec 13, 2019
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 this pull request may close these issues.

2 participants