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

Introduce chainerx/kernels/ and rename existing device "op"s to "kernel"s #6944

Merged
merged 13 commits into from
Apr 17, 2019
Merged

Introduce chainerx/kernels/ and rename existing device "op"s to "kernel"s #6944

merged 13 commits into from
Apr 17, 2019

Conversation

hvy
Copy link
Member

@hvy hvy commented Apr 16, 2019 •
edited
Loading

As the term "op"s are used for multiple purposes in ChainerX, we've decided to rename device ops to "kernels". Also, it's problematic having device ops declared in the routine headers since the device ops are of a lower concept, closer to the device than the routines. This current situation is suboptimal and is as is mostly for a historical reason. This PR does three things to remedy it.

  • Introduces chainerx_cc/chainerx/kernels/.
  • Moves device ops from routine headers to chainerx_cc/chainerx/kernels/.
  • Renames occurrences "op" to "kernel", e.g. class Op -> class Kernel and CallOp -> CallKernel.

@hvy hvy added cat:feature Implementation that introduces new interfaces. no-compat Change that disrupts backward compatibility. ChainerX Related to ChainerX. labels Apr 16, 2019
@hvy hvy added this to Ready to Review in ChainerX sprint backlog via automation Apr 16, 2019
@hvy hvy moved this from Ready to Review to In Progress in ChainerX sprint backlog Apr 16, 2019
@hvy hvy removed the no-compat Change that disrupts backward compatibility. label Apr 16, 2019
@hvy hvy marked this pull request as ready for review April 16, 2019 09:47
@hvy hvy changed the title [WIP] Introduce kernels and rename existing device ops to kernels Introduce kernels and rename existing device ops to kernels Apr 16, 2019
@hvy hvy changed the title Introduce kernels and rename existing device ops to kernels Introduce kernels and rename existing device "op"s to "kernel"s Apr 16, 2019
@hvy hvy changed the title Introduce kernels and rename existing device "op"s to "kernel"s Introduce chainerx/kernels and rename existing device "op"s to "kernel"s Apr 16, 2019
@hvy hvy changed the title Introduce chainerx/kernels and rename existing device "op"s to "kernel"s Introduce chainerx/kernels/ and rename existing device "op"s to "kernel"s Apr 16, 2019
@hvy hvy moved this from In Progress to Ready to Review in ChainerX sprint backlog Apr 16, 2019
@niboshi
Copy link
Member

niboshi commented Apr 16, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 863d05c (a0792c9):

@niboshi niboshi added st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. cat:code-fix Code refactoring that does not change the behavior. and removed cat:feature Implementation that introduces new interfaces. labels Apr 16, 2019
@niboshi niboshi moved this from Ready to Review to Waiting for CI in ChainerX sprint backlog Apr 16, 2019
@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 863d05c (a0792c9):

@hvy
Copy link
Member Author

hvy commented Apr 17, 2019

Jenkins, test this please.

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 863d05c (a0792c9):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 863d05c, target branch master) failed with status FAILURE.

@kshitij12345 kshitij12345 mentioned this pull request Apr 17, 2019
Merged
@chainer-ci
Copy link
Member

Jenkins CI test (for commit 863d05c, target branch master) failed with status FAILURE.

@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 863d05c (b65b15a):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 863d05c, target branch master) succeeded!

@niboshi
Copy link
Member

niboshi commented Apr 17, 2019

LGTM

@niboshi niboshi merged commit 299637b into chainer:master Apr 17, 2019
ChainerX sprint backlog automation moved this from Waiting for CI to Done Apr 17, 2019
@niboshi niboshi added this to the v7.0.0a1 milestone Apr 17, 2019
@hvy hvy deleted the kernels branch April 17, 2019 06:50
@hvy hvy removed the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Apr 17, 2019
@hvy hvy mentioned this pull request Apr 17, 2019
Merged
@niboshi niboshi self-assigned this Apr 17, 2019
@hvy hvy mentioned this pull request Apr 18, 2019
Closed
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@niboshi niboshi

Labels
cat:code-fix Code refactoring that does not change the behavior. ChainerX Related to ChainerX.
Projects
ChainerX sprint backlog
  
Done
Milestone
v7.0.0a1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hvy @niboshi @pfn-ci-bot @chainer-ci