Skip to content

Revert "docs(install): document cuTENSOR/cuQuantum as required for CUDA builds (#1130)" - #1134

Merged
yingjerkao merged 1 commit into
masterfrom
revert/1130-docs-install
Jul 29, 2026
Merged

Revert "docs(install): document cuTENSOR/cuQuantum as required for CUDA builds (#1130)"#1134
yingjerkao merged 1 commit into
masterfrom
revert/1130-docs-install

Conversation

@yingjerkao

Copy link
Copy Markdown
Collaborator

Reverts #1130 (merged as a0ddbbe2) at the maintainer's request.

docs/source/adv_install.rst returns 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:

X Pull request #1130 can't be reopened because it was already merged

So the merge is undone here instead. The original branch docs/1970-cuda-install-options has been restored at its original head 910e8261, 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:

Refs #1130, #970.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@yingjerkao
yingjerkao merged commit ae0f291 into master Jul 29, 2026
16 checks passed
@yingjerkao
yingjerkao deleted the revert/1130-docs-install branch July 29, 2026 03:32
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.06%. Comparing base (cf539f0) to head (be0ba6c).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

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           
Flag Coverage Δ
cpp 73.18% <ø> (ø)
python 64.13% <ø> (ø)

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

Components Coverage Δ
C++ backend 72.54% <ø> (ø)
Python bindings 76.75% <ø> (ø)
Python package 64.13% <ø> (ø)

Continue to review full report in Codecov by Harness.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant