Skip to content
This repository has been archived by the owner on Sep 25, 2023. It is now read-only.

[REVIEW] Implemented convolve1d{2/3}o 'valid', needed={tests,benchmarks,optimizations} #270

Merged
merged 5 commits into from
Nov 18, 2020
Merged

[REVIEW] Implemented convolve1d{2/3}o 'valid', needed={tests,benchmarks,optimizations} #270

merged 5 commits into from
Nov 18, 2020

Conversation

randompast
Copy link
Contributor

…eded={tests,validations,benchmarks,optimizations}

@randompast
Implemented convolve{1d2o,1d3o} mode='valid', shape={square,cube}, ne…
2cca8c0 
…eded={tests,validations,benchmarks,optimizations}
@randompast randompast requested a review from a team as a code owner October 19, 2020 04:08
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

@randompast randompast changed the title Implemented convolve1d{2/3}o 'valid, needed={tests,benchmarks,optimizations} Implemented convolve1d{2/3}o 'valid', needed={tests,benchmarks,optimizations} Oct 19, 2020
@randompast randompast mentioned this pull request Oct 19, 2020
Closed
@awthomp
Copy link
Member

awthomp commented Oct 19, 2020

ok to test

1 similar comment
@BradReesWork
Copy link
Member

ok to test

@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@BradReesWork BradReesWork added this to the 0.17 milestone Oct 19, 2020
@BradReesWork BradReesWork added the 3 - Ready for Review Ready for review by team label Oct 19, 2020
@randompast
Copy link
Contributor Author

I see the note "CHANGELOG.md has not been updated!"

Really, it's just as the commit message:
"Implemented (valid) second and third order convolve as convolve1d2o and convolve1d3o."

@awthomp
Copy link
Member

awthomp commented Oct 23, 2020

Hi @randompast -- just getting some time to look at this PR! To kick off CI/CD, could you please update the changelog and push to the branch you used to submit this PR?

From the head cusignal directory, edit CHANGELOG.md and under 0.17 Improvements, type something like: - PR #270 - Implement higher order convolutions.

@randompast
Copy link
Contributor Author

Updated, thanks for the feedback @awthomp!

@BradReesWork BradReesWork changed the title Implemented convolve1d{2/3}o 'valid', needed={tests,benchmarks,optimizations} [REVIEW] Implemented convolve1d{2/3}o 'valid', needed={tests,benchmarks,optimizations} Nov 11, 2020
@BradReesWork BradReesWork added the 4 - Waiting on Author Waiting for author to respond to review label Nov 17, 2020
@BradReesWork BradReesWork added 6 - Okay to Merge improvement Improvement / enhancement to an existing function and removed 3 - Ready for Review Ready for review by team 4 - Waiting on Author Waiting for author to respond to review labels Nov 18, 2020
@BradReesWork
Copy link
Member

@randompast this is a style issue blocing this PR:

./cusignal/convolution/convolve.py:587:1: W293 blank line contains whitespace
./cusignal/convolution/_convolution_cuda.py:638:15: W292 no newline at end of file

@BradReesWork BradReesWork merged commit 53bdc01 into rapidsai:branch-0.17 Nov 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BradReesWork BradReesWork BradReesWork approved these changes

@mnicely mnicely Awaiting requested review from mnicely

@awthomp awthomp Awaiting requested review from awthomp

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function
Projects
None yet
Milestone
0.17
Development

Successfully merging this pull request may close these issues.
4 participants
@randompast @GPUtester @awthomp @BradReesWork