[QDP][Housekeeping] Clean structure and make commands #756
Conversation
|
cc @ryankert01 |
ryankert01
left a comment
ryankert01
left a comment
There was a problem hiding this comment.
Some suggestion about usage.
qdp/DEVELOPMENT.md
Outdated
| make unit_test # Run all unit tests (Python + Rust) | ||
| make unit_test_python # Run Python tests only | ||
| make unit_test_rust # Run Rust tests only |
There was a problem hiding this comment.
nits
| make unit_test # Run all unit tests (Python + Rust) | |
| make unit_test_python # Run Python tests only | |
| make unit_test_rust # Run Rust tests only | |
| make test # Run all unit tests (Python + Rust) | |
| make test_python # Run Python tests only | |
| make test_rust # Run Rust tests only |
qdp/DEVELOPMENT.md
Outdated
| Or use the make command from the `qdp/` directory: | ||
|
|
||
| ```bash | ||
| make e2e_test |
There was a problem hiding this comment.
Our benchmarks are not stable right now, we might use different benchmark command. so I don't think we need this in make.
eg. tryout different qubitspython e2e.py --qubits 100
| run_nvtx_profile: | ||
| $(eval EXAMPLE ?= nvtx_profile) | ||
| @echo "Building example '$(EXAMPLE)' with observability features..." | ||
| cargo build -p qdp-core --example $(EXAMPLE) --features observability --release | ||
| @echo "Running '$(EXAMPLE)' with nsys profiling..." | ||
| nsys profile --trace=cuda,nvtx --force-overwrite=true -o report ./target/release/examples/$(EXAMPLE) | ||
| @echo "Showing profiling statistics..." | ||
| nsys stats --force-export=true report.nsys-rep |
There was a problem hiding this comment.
(optional) In this, we can only profile /example? Is this a big limitation?
I think it's also unnessary for make, but not sure if others think otherwise.
cc @rich7420 I'm not familiar with this. I never use /example to profile XD. I profile benchmarks directly everytime.
There was a problem hiding this comment.
Could you please show me how to profile benchmarks directly? The documentation we had profiling the example, that's why I only put example here
There was a problem hiding this comment.
I use something like this, and it's because I also want to profile every parts of the code.
nsys profile python some_benchmark.py
Could you implement a make install or something that makes it profile-able?
It should be build like cargo build -p qdp-core --example $(EXAMPLE) --features observability --release as in L46 to make profile-able.
There was a problem hiding this comment.
@ryankert01
Actually I did profile example before you told me this way.
qdp/Makefile
Outdated
| e2e_test: | ||
| @echo "Running e2e benchmark tests..." | ||
| cd qdp-python/benchmark && uv run python benchmark_e2e.py | ||
| cd qdp-python/benchmark && uv run python benchmark_dataloader_throughput.py |
There was a problem hiding this comment.
| e2e_test: | |
| @echo "Running e2e benchmark tests..." | |
| cd qdp-python/benchmark && uv run python benchmark_e2e.py | |
| cd qdp-python/benchmark && uv run python benchmark_dataloader_throughput.py | |
| install_benchmark: | |
| cd qdp/qdp-python && uv sync --group benchmark | |
| benchmark: install install_benchmark | |
| @echo "Running e2e benchmark tests..." | |
| uv run python qdp-python/benchmark/benchmark_e2e.py | |
| uv run python qdp-python/benchmark/benchmark_dataloader_throughput.py |
|
Thanks @machichima 's contribution! I tested locally and these commands works. |
|
Thanks @ryankert01 for review! I updated based on your review:
|
ryankert01
left a comment
ryankert01
left a comment
There was a problem hiding this comment.
lg, please add [QDP] prefix for the pr. Thanks for your contribution.
rich7420
changed the title
|
@machichima thanks for the patch |
|
@machichima plz resolve the conflict. |
3 participants
Purpose of PR
Improve development experience by introducing make commands to install, run tests, and do profiling.
Also move
benchmark/intoqdp-pythonand updatepyproject.tomlto install dependencies needed with different--groupargsRelated Issues or PRs
Closes #735
Changes Made
Breaking Changes
Checklist