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

Installation instructions for cuQuantum backend #534

Merged
merged 3 commits into from
Jan 19, 2022
Merged

Conversation

andrea-pasquale
Copy link
Contributor

This PR is related to the cuQuantum backend from qiboteam/qibojit#57.
I've updated the installation instructions and I've added cuQuantum along the other platforms available in qibojit.
Let me know what you think.

@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #534 (c1c915d) into master (0cf3937) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #534   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           85        85           
  Lines        12439     12439           
=========================================
  Hits         12439     12439           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cf3937...c1c915d. Read the comment docs.

Copy link
Contributor

@mlazzarin mlazzarin left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me, only one comment.

Comment on lines 113 to 115
dependencies except `cupy <https://cupy.dev/>`_ and
`cuQuantum <https://developer.nvidia.com/cuquantum-sdk>`_
which are required for GPU acceleration. Please install `cupy <https://cupy.dev/>`_ by following the
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we clarify that our cuquantum backend is an alternative GPU implementation, so that installing it is not required to have GPU acceleration (in contrast to cupy)?

Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

I agree with Marco's comment, we can add a sentence where we say that cuquantum is optional. Other than that it looks good, thanks.

In #533 I document how one can switch between cupy and cuquantum using qibo.set_backend(). At some point we should also document the default switch using environment variables, along with other environment variables we have in qibo. I'm not sure if this should be done here or in a different PR.

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
@andrea-pasquale
Copy link
Contributor Author

Thank you both for the feedback!
As @stavros11 suggested, I've added a sentence both in the pip and conda note saying that cuQuantum is an optional dependency. Let me know if there is something else to change.

In #533 I document how one can switch between cupy and cuquantum using qibo.set_backend(). At some point we should also document the default switch using environment variables, along with other environment variables we have in qibo. I'm not sure if this should be done here or in a different PR.

If you want to add it directly to #533 I am fine with it. Otherwise I can add everything related to the documentation directly here.

@scarrazza
Copy link
Member

@andrea-pasquale thanks, if you include the and/or this might increase readability.

Co-authored-by: Stefano Carrazza <scarrazza@users.noreply.github.com>
@andrea-pasquale
Copy link
Contributor Author

@andrea-pasquale thanks, if you include the and/or this might increase readability.

Thanks for the feedback. I've added your suggestions.

@scarrazza scarrazza changed the base branch from master to setplatform January 19, 2022 09:07
@scarrazza scarrazza merged commit e3d03f7 into setplatform Jan 19, 2022
@scarrazza scarrazza deleted the doc_cuquantum branch January 20, 2022 14:47
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.

4 participants