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

Removed move_to_XXX kernels and COO::transpose #288

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

thoasm
Copy link
Member

@thoasm thoasm commented Apr 3, 2019

Removed unused move_to_XXX kernels from Dense and Csr. All calls to move_to(Mtx *), which previously used the mentioned kernels, now use convert_to_XXX kernels.

Since we did not implement any transpose functionality in Coo, the inheritance from Transposable and all related kernels were removed.

Additionally, the tests for Coo, Csr and Dense were modified as follows:

  • Made include order compliant to guidelines
  • Newlines in the tests are also according to guidelines
  • Added MoveTo tests where they were not present
  • Added tests for Dense conversions (used already generated variables)

The conversion from Dense to Hybrid with OpenMP was also slightly improved.

Takes care of a part of #286.

@thoasm thoasm added reg:testing This is related to testing. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:openmp This is related to the OpenMP module. mod:reference This is related to the reference module. type:matrix-format This is related to the Matrix formats 1:ST:ready-for-review This PR is ready for review labels Apr 3, 2019
@thoasm thoasm added this to the Ginkgo 1.0.0 release milestone Apr 3, 2019
@thoasm thoasm self-assigned this Apr 3, 2019
@thoasm thoasm force-pushed the matrix_replace_move_to branch 3 times, most recently from 463eacd to c027ab3 Compare April 4, 2019 13:23
Copy link
Member

@tcojean tcojean left a comment

Choose a reason for hiding this comment

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

LGTM. A lot of good finds. I have very minor comments which are optional.

cuda/test/matrix/csr_kernels.cpp Outdated Show resolved Hide resolved
cuda/test/matrix/dense_kernels.cpp Show resolved Hide resolved
omp/matrix/dense_kernels.cpp Outdated Show resolved Hide resolved
omp/matrix/dense_kernels.cpp Show resolved Hide resolved
omp/test/matrix/dense_kernels.cpp Outdated Show resolved Hide resolved
Copy link
Member

@pratikvn pratikvn left a comment

Choose a reason for hiding this comment

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

LGTM!

core/matrix/csr.cpp Outdated Show resolved Hide resolved
core/matrix/dense.cpp Outdated Show resolved Hide resolved
@thoasm thoasm added the 1:ST:do-not-merge Please do not merge PR this yet. label Apr 4, 2019
Removed unused `move_to_XXX` kernels from Dense and Csr. All calls to
`move_to(Mtx *)`, which previously used the mentioned kernels, now use
`convert_to_XXX` kernels.

Since we did not implement any transpose functionality in Coo, the
inheritance from `Transposable` and all related kernels were removed.

Additionally, the tests for Coo, Csr & Dense were modified as follows:
- Made include order compliant to guidelines
- Newlines in the tests are also according to guidelines
- Added MoveTo tests where they were not present
- Added tests for Dense conversions (used already generated variables)

The conversion from Dense to Hybrid with OpenMP was also slightly
improved.
@thoasm thoasm added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:do-not-merge Please do not merge PR this yet. 1:ST:ready-for-review This PR is ready for review labels Apr 4, 2019
@thoasm thoasm merged commit ef6df57 into develop Apr 4, 2019
@thoasm thoasm deleted the matrix_replace_move_to branch April 4, 2019 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1:ST:ready-to-merge This PR is ready to merge. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:openmp This is related to the OpenMP module. mod:reference This is related to the reference module. reg:testing This is related to testing. type:matrix-format This is related to the Matrix formats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants