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

Rust code #77

Open
wants to merge 8 commits into
base: development
Choose a base branch
from
Open

Rust code #77

wants to merge 8 commits into from

Conversation

ciaranra
Copy link
Member

@ciaranra ciaranra commented Sep 16, 2024

Including Rust code for stabilizer simulator.

TODO:

  • Have CI workflow that checks Rust tests and formatting
  • Update Makefiles to build etc. both Rust and Python projects given restructuring
  • Have CI workflow that builds and install pecos-rslib and tests pecos-rslib
  • Have CI workflow that tests the usage of pecos-rslib from Python PECOS
  • Add PyO3 wrapping for Rust simulators and make available to Python via pecos-rslib
  • Update tests to test Rust simulators used in Python PECOS
  • Add RSSparseSim to Python test_random_circuits (likely need to add force_outcome measurement)
  • Add Rust tests for the newly added gates for the Rust stabilizer sim
  • Update/modify various Readmes as appropriate...
  • Use symlinks to point to the appropriate legal and Readme stuff
  • Have CI workflow that is generates sdists and wheels for all the Python packages
  • Add version consistency checks for Python packages and Rust crates
  • Update workflow to save artifacts only on success merges, direct pushes, or py- tags in main branches
  • Update README.md if needed

@ciaranra ciaranra marked this pull request as draft September 27, 2024 22:39
@ciaranra ciaranra mentioned this pull request Sep 29, 2024
@ciaranra ciaranra added the enhancement New feature or request label Oct 1, 2024
@ciaranra ciaranra marked this pull request as ready for review October 14, 2024 16:59
@peter-campora
Copy link
Collaborator

Oh boy CRA, this is a monster PR. I think it would help if you have some examples of the kinds of tests you would like the PR reviewers to run on this

@ciaranra
Copy link
Member Author

Hehe, yeah it is a monster. Let me single out some smaller stuff for people to maybe concentrate on if they want to...

@ciaranra
Copy link
Member Author

If you are up for reviewing this monster of a PR, here are some things that you can take a look at:

  • Makefile: After reading the README and checking out the Makefile is it clear what to do? Do the relevant targets make sense? Maybe take the makefile out for a spin and see if you can build and test with it?
  • README: Do you think there is anything glaringly missing for users or developers?
  • Workflows: New workflows have been added to test and build both the Rust and Python code: .github/workflows. If you want you can see if all the testing and linting look right. The workflows look like they are doing their jobs properly... python-release.yml is a bit of a beast, but it is mainly a utility for building distributions to be uploaded to pypi
  • tests... the tests are fairly extensive and probably more for people who are comfortable with stabilizer simulations. If you want to take a look at them... they are in crates/pecos-qsims/src/sparse_stab.rs (at the bottom of the file), in here python/tests/pecos/integration/state_sim_tests/test_stab_sims/, and here python/tests/pecos/integration/test_random_circuits.py. The Rust stabilizer simulator is probably better tested then the Python one. lol
  • General structure: Does the general approach and structure of files for this monolithic repo seem okay?

@ciaranra
Copy link
Member Author

ciaranra commented Oct 16, 2024

New TODO:

  • Add license headers to all code
  • Remove Rust code unless it is supporting the specific stabilizer sim implementation that is fully baked out (@peter-campora is keeping me honest... sigh... lol)

Copy link
Collaborator

@qartik qartik left a comment

Choose a reason for hiding this comment

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

Mostly just tried to work through the development section of the README.

make test-all gave me:

======== short test summary info ========
FAILED docs/api_guide/circuit_runners.rst::circuit_runners.rst
FAILED docs/api_guide/decoders.rst::decoders.rst
FAILED docs/api_guide/error_generators.rst::error_generators.rst
FAILED docs/api_guide/qeccs.rst::qeccs.rst
FAILED docs/api_guide/simulators.rst::simulators.rst
FAILED docs/examples/creating_qecc_class.rst::creating_qecc_class.rst
FAILED docs/examples/stab_code_verification.rst::stab_code_verification.rst
======== 7 failed, 2 passed, 1 warning in 0.82s ========

metadeps: upgrade-pip ## Install extra dependencies used to develop/build this package
$(VENV_BIN)/pip install -U build pip-tools pre-commit wheel pytest hypothesis
.PHONY: venv
venv: ## Build Python virtual environment and install requirements
Copy link
Collaborator

Choose a reason for hiding this comment

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

make venv shouldn't modify python/quantum-pecos/requirements.txt. It currently adds any trusted hosts from my local config to the file. Also, the following change induced by this step looks incorrect:

-pecos-rslib==0.6.0.dev6
+pecos-rslib==version = 0.6.0.dev6

README.md Outdated
Comment on lines 104 to 105
make venv
make build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
make venv
make build
make build

It seems build calls the venv so the first one is unnecessary.

I know you like the automation from make files, but none of these steps without an explicit source .venv/bin/activate leave the developer inside a venv, so it's best to make these steps explicit.

I tend to leave these steps in each Python project I manage:

python -m venv .venv
source .venv/bin/activate
pip install -U pip setuptools
pip install -r requirements.txt
pre-commit install

README.md Outdated Show resolved Hide resolved
README.md Outdated

4. Run pre-commit (after [installing it](https://pre-commit.com/))
```sh
make pre-commit
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wish there was a single step to set up the virtual environment and not all make commands be calling make venv, which can be slow and annoying.

Makefile Outdated
# Building development environments
# ---------------------------------
.PHONY: build
build: venv ## Compile and install for development
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fails for me with:

RROR: Could not find a version that satisfies the requirement pecos-rslib==0.6.0.dev6 (from quantum-pecos) (from versions: none)
ERROR: No matching distribution found for pecos-rslib==0.6.0.dev6

I am assuming I need to make requirements first? I will try that and then retry make build

Copy link
Collaborator

@peter-campora peter-campora Oct 16, 2024

Choose a reason for hiding this comment

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

Hmmm same error, but looking at the fuller dump, I'm very confused because it looks like the package is being installed and then not used:

📦 Including license file "/home/peter/Downloads/PECOS-feat-rust/python/pecos-rslib/LICENSE"
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.10 at /home/peter/Downloads/rust-pecos/bin/python
📡 Using build options features from pyproject.toml
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s
📦 Built wheel for CPython 3.10 to /tmp/.tmpzQTl39/pecos_rslib-0.6.0.dev6-cp310-cp310-linux_x86_64.whl
✏️  Setting installed package as editable
🛠 Installed pecos-rslib-0.6.0.dev6
Looking in indexes: snip
Obtaining file:///home/peter/Downloads/PECOS-feat-rust/python/quantum-pecos
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
INFO: pip is looking at multiple versions of quantum-pecos to determine which version is compatible with other requirements. This could take a while.
ERROR: Could not find a version that satisfies the requirement pecos-rslib==0.6.0.dev6 (from quantum-pecos) (from versions: none)
ERROR: No matching distribution found for pecos-rslib==0.6.0.dev6
make: *** [Makefile:75: build] Error 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

Weirdly, if I define my own virtual environment and then perform the commands from make build by hand, everything seem shappy.

Makefile Outdated
clean: ## Clean up caches and build artifacts
-rm -rf *.egg-info dist build docs/_build .pytest_cache/ .ruff_cache/
.PHONY: test-all
test-all: rstest pytest pytest-dep pydoctest ## Run all tests. ASSUMES: previous build command
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm getting a lot errors of this pecos is not a package form:

___________________________________________ ERROR collecting pecos/unit/test_blocks.py ____________________________________________
ImportError while importing test module '/home/peter/Downloads/PECOS-feat-rust/python/tests/pecos/unit/test_blocks.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
pecos/unit/test_blocks.py:13: in <module>
    from pecos.classical_interpreters.phir_classical_interpreter import (
E   ModuleNotFoundError: No module named 'pecos.classical_interpreters'; 'pecos' is not a package

Full test failure list:

ERROR pecos/integration/example_tests/test_basic_logical_sim.py
ERROR pecos/integration/example_tests/test_finding_threshold.py
ERROR pecos/integration/example_tests/test_recovery.py
ERROR pecos/integration/state_sim_tests/test_cointoss.py
ERROR pecos/integration/state_sim_tests/test_stab_sims/test_gate_init.py
ERROR pecos/integration/state_sim_tests/test_stab_sims/test_gate_one_qubit.py
ERROR pecos/integration/state_sim_tests/test_stab_sims/test_gate_two_qubit.py
ERROR pecos/integration/state_sim_tests/test_statevec.py
ERROR pecos/integration/test_phir.py
ERROR pecos/integration/test_phir_64_bit.py
ERROR pecos/integration/test_phir_setting_cregs.py
ERROR pecos/integration/test_quantum_circuits.py
ERROR pecos/integration/test_random_circuits.py
ERROR pecos/regression/test_qasm/examples/test_logical_steane_code_program.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_measures.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_preps.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_rots.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_sq_face_rots.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_sq_hadamards.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_sq_noncliffords.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_sq_paulis.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_sq_sqrt_paulis.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_tq_cliffords.py
ERROR pecos/regression/test_qasm/pecos/qeclib/qubit/test_tq_noncliffords.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/decoders/test_lookup.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/gates_sq/test_face_rots.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/gates_sq/test_hadamards.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/gates_sq/test_paulis.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/gates_sq/test_sqrt_paulis.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/gates_tq/test_transversal_tq.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/meas/test_destructive_meas.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/meas/test_measure_x.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/meas/test_measure_z.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/preps/test_encoding_circ.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/preps/test_pauli_states.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/preps/test_plus_h_state.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/preps/test_t_plus_state.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/qec/test_qec_3parallel.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/syn_extract/test_six_check_nonflagging.py
ERROR pecos/regression/test_qasm/pecos/qeclib/steane/syn_extract/test_three_parallel_flagging.py
ERROR pecos/regression/test_qasm/random_cases/test_control_flow.py
ERROR pecos/regression/test_qasm/random_cases/test_slr_phys.py
ERROR pecos/unit/reps/pymir/test_name_resolver.py
ERROR pecos/unit/test_binarray.py
ERROR pecos/unit/test_blocks.py

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you follow this note? ## Run all tests. ASSUMES: previous build command

Although, if you are having issues... probably need to find another solution. :D

@ciaranra ciaranra force-pushed the feat-rust branch 11 times, most recently from 7bc6044 to fd69fd6 Compare October 17, 2024 02:22
@ciaranra
Copy link
Member Author

Mostly just tried to work through the development section of the README.

make test-all gave me:

======== short test summary info ========
FAILED docs/api_guide/circuit_runners.rst::circuit_runners.rst
FAILED docs/api_guide/decoders.rst::decoders.rst
FAILED docs/api_guide/error_generators.rst::error_generators.rst
FAILED docs/api_guide/qeccs.rst::qeccs.rst
FAILED docs/api_guide/simulators.rst::simulators.rst
FAILED docs/examples/creating_qecc_class.rst::creating_qecc_class.rst
FAILED docs/examples/stab_code_verification.rst::stab_code_verification.rst
======== 7 failed, 2 passed, 1 warning in 0.82s ========

This should be resolved now. Just removing doc tests for now.

@ciaranra
Copy link
Member Author

Alright, @qartik & @peter-campora. I have updated the makefile to make the .venv more of a "first class citizen" and updated the README appropriately (hopefully). Let me know if you still have issues.

I still need to figure out what is going on with this:

-pecos-rslib==0.6.0.dev6
+pecos-rslib==version = 0.6.0.dev6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants