Revert "docs(install): document cuTENSOR/cuQuantum as required for CUDA builds (#1130)" - #1134
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be0ba6cee3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (and ``mkl-cuda`` / ``debug-openblas-cuda`` / ``debug-mkl-cuda``, which inherit it) | ||
| sets ``USE_CUDA``, ``USE_CUTENSOR`` and ``USE_CUQUANTUM`` to ``ON``, and so do the | ||
| GPU CI job and the published ``cytnx-cuda`` wheels. | ||
| $conda install -c nvidia cuda |
There was a problem hiding this comment.
Install the dependencies enabled by CUDA presets
A conda user who follows this GPU step and then uses the documented preset workflow cannot configure a CUDA build: openblas-cuda and mkl-cuda force both USE_CUTENSOR and USE_CUQUANTUM on in CMakePresets.json, while CMakeLists.txt aborts unless their packages and CUTENSOR_ROOT/CUQUANTUM_ROOT are available. Presenting cuTENSOR and cuQuantum as optional and installing only cuda therefore leaves the shipped CUDA presets unusable; restore the required packages and root-variable setup or explain how to disable those preset options.
Useful? React with 👍 / 👎.
| $conda install -c nvidia cutensor | ||
|
|
||
| .. code-block:: shell | ||
| Similarly, cuqauantum (compile option -DUSE_CUTENSOR=ON), requires: |
There was a problem hiding this comment.
Use the cuQuantum option in the cuQuantum instructions
For users configuring without a preset, this paragraph tells them to enable USE_CUTENSOR when they want cuQuantum. That option enables a different dependency and never activates cuQuantum functionality; the documented compile option here must be -DUSE_CUQUANTUM=ON.
Useful? React with 👍 / 👎.
| | options | default | description | | ||
| +------------------------+-------------------+------------------------------------+ | ||
| | -DCMAKE_INSTALL_PREFIX | ~/.local/cytnx | Install destination of the library | | ||
| | -DCMAKE_INSTALL_PREFIX | /usr/local/cytnx | Install destination of the library | |
There was a problem hiding this comment.
Report the configured install prefix
The stated default disagrees with both CMakeLists.txt, which initializes CMAKE_INSTALL_PREFIX to ~/.local/cytnx, and the default preset in CMakePresets.json. Users who omit the option will therefore look under /usr/local/cytnx even though the project installs under their home directory.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1134 +/- ##
=======================================
Coverage 73.06% 73.06%
=======================================
Files 224 224
Lines 27642 27642
Branches 71 71
=======================================
Hits 20196 20196
Misses 7425 7425
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Reverts #1130 (merged as
a0ddbbe2) at the maintainer's request.docs/source/adv_install.rstreturns to its pre-#1130 state: 71 lines removed, 9 restored — the exact inverse of the merge. No other file is touched, and nothing else on master depends on it.Why this is a revert PR and not a reopen
#1130 was squash-merged, and GitHub cannot reopen a merged pull request:
So the merge is undone here instead. The original branch
docs/1970-cuda-install-optionshas been restored at its original head910e8261, so none of the work is lost and it can be carried into a fresh PR whenever the content is wanted again.What #1130 had changed
For reference, so it can be reconstructed or reworked:
CUTENSOR_ROOT/CUQUANTUM_ROOTBACKEND_TORCHrow~/.local/cytnx(the page said/usr/local/cytnx)-DUSE_CUTENSOR=ON, and a "cuqauantum" typoRefs #1130, #970.