Skip to content

Handle PCI bus ID#2531

Merged
niboshi merged 18 commits intocupy:masterfrom
jameshclrk:feature/device-pci-bus-id
Oct 11, 2019
Merged

Handle PCI bus ID#2531
niboshi merged 18 commits intocupy:masterfrom
jameshclrk:feature/device-pci-bus-id

Conversation

@jameshclrk
Copy link
Copy Markdown
Contributor

@jameshclrk jameshclrk commented Oct 6, 2019 •
edited
Loading

From the issue #2440.

This PR adds two features:

  • Get a new Device by PCI Bus ID
  • Get the PCI Bus ID from a Device

I'm not sure what tests I should add, I have:

  • Check the returned PCI bus ID matches a specific format
  • Ensure a device created with the PCI bus ID of another Device is the same device

niboshi
niboshi reviewed Oct 7, 2019
View reviewed changes
niboshi
niboshi reviewed Oct 7, 2019
View reviewed changes
@niboshi niboshi added st:awaiting-author Awaiting response from author cat:feature New features/APIs labels Oct 7, 2019
@jameshclrk
Copy link
Copy Markdown
Contributor Author

jameshclrk commented Oct 7, 2019

I have addressed the comments, however I'm not sure why the Travis builds are failing. I have tried on two local machines and CuPy builds

@leofang
Copy link
Copy Markdown
Member

leofang commented Oct 7, 2019

@jameshclrk It's because on the CIs, it needs to recognize the CUDA APIs in the absence of CUDA toolkit. You need to add "fake" wrappers for the two runtime APIs you cover in this PR to cupy/cuda/cupy_cuda.h for testing Read the Docs. Search and take a look at the cuda_runtime.h section in that header file, and you'll know what to do.

@jameshclrk
Copy link
Copy Markdown
Contributor Author

jameshclrk commented Oct 7, 2019

Thanks, I added the functions there.

@niboshi niboshi removed the st:awaiting-author Awaiting response from author label Oct 8, 2019
niboshi
niboshi requested changes Oct 8, 2019
View reviewed changes
niboshi
niboshi requested changes Oct 10, 2019
View reviewed changes
@jameshclrk
Copy link
Copy Markdown
Contributor Author

jameshclrk commented Oct 10, 2019

I've pushed those latest changes. Thanks for your patience reviewing.

niboshi
niboshi approved these changes Oct 10, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@niboshi niboshi left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@niboshi
Copy link
Copy Markdown
Member

niboshi commented Oct 10, 2019

Jenkins, test this please

@pfn-ci-bot
Copy link
Copy Markdown
Collaborator

pfn-ci-bot commented Oct 10, 2019

Successfully created a job for commit 1438d14:

@niboshi niboshi added this to the v7.0.0rc1 milestone Oct 10, 2019
@niboshi niboshi added the st:test-and-merge (deprecated) Ready to merge after test pass. label Oct 10, 2019
@chainer-ci
Copy link
Copy Markdown
Member

chainer-ci commented Oct 10, 2019

Jenkins CI test (for commit 1438d14, target branch master) succeeded!

@niboshi niboshi merged commit d3e35cc into cupy:master Oct 11, 2019
@leofang leofang mentioned this pull request Oct 18, 2019
Closed
@kmaehashi kmaehashi mentioned this pull request Nov 13, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leofang leofang leofang left review comments

@niboshi niboshi niboshi approved these changes

Assignees

@niboshi niboshi

Labels

cat:feature New features/APIs st:test-and-merge (deprecated) Ready to merge after test pass.

Projects

None yet

Milestone

v7.0.0rc1

Development

Successfully merging this pull request may close these issues.

5 participants

@jameshclrk @leofang @niboshi @pfn-ci-bot @chainer-ci