Skip to content

Conversation

@Jonhas-qtm
Copy link
Collaborator

@Jonhas-qtm Jonhas-qtm commented Jul 1, 2025

This patch is focused on implementing RNG platform calls in PECOs. Accompanying PR is https://github.com/CQCL/qemulator/pull/11

@Jonhas-qtm Jonhas-qtm requested a review from ciaranra as a code owner July 1, 2025 07:40
@ciaranra
Copy link
Member

ciaranra commented Jul 3, 2025

I need to look through this more closely but as a really quick comment, the structure I had in mind for different language libraries was to have fairly clear, specialized directories for language types, e.g., crates/ for Rust, python/ for Python packages. It seems like either the pecos_rng_pcg could be split up in something like c/ or clibs/ and a Python package in python/ or for now move it to python/pecos-rng-pcg/ if we should just think of this as a Python package for now.

Also generally, the style I have been going with is using kebab case for package folder (so like pecos-name but imported like pecos_name). See, e.g., python/pecos-rsllib/.

@ciaranra
Copy link
Member

ciaranra commented Jul 6, 2025

Just double checking, instead of adding our own PCG wrapper should we be using ones that already exist like: https://github.com/Bladieblah/PyPCG or https://github.com/afnanenayet/pcg-rs? Have you properly evaluated already existing solutions? Is there an advantage to rolling our own?

@ciaranra
Copy link
Member

ciaranra commented Jul 8, 2025

I am having issues with this installing when I run make dev:

make buildrng
make[1]: Entering directory '~/Repos/PECOS'
Building and installing RNG library...
uv pip install nanobind
Audited 1 package in 1ms
nanobind_DIR="python3 -m nanobind --include_dir"
cd clibs/pecos-rng && mkdir build && cd build/ && cmake .. && cmake --build . && cd .. && uv pip install .
-- The C compiler identification is GNU 11.4.0
-- The CXX compiler identification is GNU 11.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Python: /usr/bin/python3.10 (found version "3.10.12") found components: Interpreter Development.Module 
/usr/bin/python3.10: No module named nanobind
CMake Error at CMakeLists.txt:27 (find_package):
  Could not find a package configuration file provided by "nanobind" with any
  of the following names:

    nanobindConfig.cmake
    nanobind-config.cmake

  Add the installation prefix of "nanobind" to CMAKE_PREFIX_PATH or set
  "nanobind_DIR" to a directory containing one of the above files.  If
  "nanobind" provides a separate development package or SDK, be sure it has
  been installed.


-- Configuring incomplete, errors occurred!

One thing I am suspicious of is that it seems to be looking at my system Python when it should be looking at my uv Python

@ciaranra
Copy link
Member

ciaranra commented Jul 8, 2025

I just pushed a potential fix. @Jonhas-qtm please revert or change any of it if it looks wrong.

@Jonhas-qtm
Copy link
Collaborator Author

I just pushed a potential fix. @Jonhas-qtm please revert or change any of it if it looks wrong.

Looks good!

@ciaranra
Copy link
Member

ciaranra commented Jul 8, 2025

LGTM

@ciaranra ciaranra merged commit 597324e into dev Jul 8, 2025
39 checks passed
@ciaranra ciaranra deleted the feature/RNGSupport branch July 8, 2025 19:14
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.

3 participants