Skip to content

Upgrade to use C++20. - #43

Merged
hzhangxyz merged 1 commit into
mainfrom
dev/upgrade-to-c++20
Jun 17, 2025
Merged

Upgrade to use C++20.#43
hzhangxyz merged 1 commit into
mainfrom
dev/upgrade-to-c++20

Conversation

@hzhangxyz

@hzhangxyz hzhangxyz commented Jun 17, 2025

Copy link
Copy Markdown
Member

Description

Upgrade to use C++20.

Closes: #40
Closes: #36

Checklist:

Closes: #40
Closes: #36
@hzhangxyz
hzhangxyz requested review from CuSO4Deposit and Copilot and removed request for Copilot June 17, 2025 08:49

Copilot AI 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.

Pull Request Overview

This PR updates the build configuration to target C++20 and modernizes tensor accessor calls in the C++ extension.

  • Adds -std=c++20 to both host and CUDA compilation flags in the Python loader
  • Removes redundant template disambiguator on .accessor<…>() calls in the C++ extension

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
qmb/hamiltonian.py Append -std=c++20 to extra C++ and CUDA compiler flags
qmb/_hamiltonian.cpp Drop unnecessary template keyword before .accessor<…>()
Comments suppressed due to low confidence (1)

qmb/hamiltonian.py:34

  • [nitpick] Consider adding a compiler feature check or fallback to C++17 to avoid build failures on systems that lack full C++20 support.
"-std=c++20"]

@hzhangxyz
hzhangxyz requested review from stevapple and removed request for CuSO4Deposit June 17, 2025 08:55

@stevapple stevapple 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.

LGTM

Comment thread qmb/_hamiltonian.cpp
auto coef_accessor = coef.template accessor<double, 2>();
auto site_accessor = site.accessor<std::int16_t, 2>();
auto kind_accessor = kind.accessor<std::uint8_t, 2>();
auto coef_accessor = coef.accessor<double, 2>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

我就不作校验了hhh最好是有 CI 能保证构建成功,回头可以看看有没有什么办法接入 GitHub

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

确实可以,搜到了这个 https://github.com/marketplace/actions/cuda-toolkit 。理论上,可以设置一个matrix,不同python、不同pytorch、不同cuda。但是1、github action上只能编译,不能运行一个哪怕简单的例子;2、量大不适合放github action里。

我平时是commit前跑一下 python -m qmb precompile openfermion -PH2 这个东西会在编译一个试试,并跑一个很简单的例子。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

哦对了,等 #42 merge 了, 目前写了的 pytest 可以直接跳过cuda部分,只测试cpu版本的。

@hzhangxyz
hzhangxyz merged commit 2869340 into main Jun 17, 2025
8 checks passed
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.

💡 [REQUEST] - Upgrade to C++20. 🐛 [BUG] - CUDA Compilation Errors after Upgrading to cuda 12.9

3 participants