Start experimental simulators feature #709
Conversation
swernli
left a comment
swernli
left a comment
There was a problem hiding this comment.
The changes look good! I have a few more questions/feedback below and one typo, but otherwise I feel good about signging off. Thanks for separating out the QIR part for now, that made this much easier to review.
| [Newtonsoft.Json.JsonConverter(typeof(DelegatedConverter<NoiseModel>))] | ||
| public class NoiseModel | ||
| { | ||
| [JsonPropertyName("initial_state")] |
There was a problem hiding this comment.
Super nitpicky: I personally am not a fan of using property aliases if the JSON object is one we design and control, but I understand the benefit when it allows bridging between two languages with different naming conventions. That said, it can sometimes cause confusion if the JSON name and the code name deviate too much and future developers get confused about which property goes where, but these names follow a nice pattern and correlate well so that seems unlikely to occur here.
There was a problem hiding this comment.
Not that nitpicky, it's a good point! I think in this case, there will necessarily be some friction in that Rust and C# use different naming conventions such that either side will be "wrong" in some sense, requiring us to explicitly specify property names on at least one side of the interoperability boundary.
We ran into something similar with Microsoft.Jupyter.Core, where the JSON property names are most definitely not under our control, and all follow snake_case.
I'm not sure of the best solution here other than making sure to follow the pattern explicitly in core review, or to use a custom naming policy in System.Text.Json (https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-customize-properties#use-a-custom-json-property-naming-policy).
src/Simulation/Simulators/OpenSystemsSimulator/OpenSystemsSimulator.cs
Outdated
Show resolved
Hide resolved
| | Out-File -FilePath opensim_results.xml -Encoding utf8NoBOM | ||
| } else { | ||
| # Outside of CI, show human-readable output. | ||
| cargo +nightly test |
There was a problem hiding this comment.
Since the build-qdk-sim-rs.ps1 includes a call to cargo clean is this going to require rebuilding the library before it can run the test? While the Rust build is not very long, I worry this pattern might mean it gets run twice in the pipeline (and maybe locally too).
There was a problem hiding this comment.
I've gated the cargo clean call so that it doesn't run in local builds, as per your earlier feedback. That said, I agree that in CI builds this will cause the crate to get built twice. I don't know how to avoid that in our current e2e builds given the hard disk space limitations and the order in which we call build and test commands. Would love to get @anpaz's input as to what may be a good way of cutting down on extraneous builds may be.
anpaz
left a comment
anpaz
left a comment
There was a problem hiding this comment.
Looks great! Have a couple of general comments:
- As I mentioned somewhere else, I'm not sure this should be called "experimental". I would name this something less scary/more catchy:
powertools,toybox,sandbox. - I'm not sure this should be under an
Experimentalnamespace. It does make it explicit, but switching to non-experimental will make it harder. - I highly encourage you to write some simple tests for the qdk e2e tests, to make sure the new simulators run successfully on all environments (OS/conda/docker/etc)
- Is it possible to configure the noisy simulator to run without noise? I think it would be important to have a test for each intrinsic operation that makes sure every operation applies the corresponding correctly; we have something like that for the FullState simulator, so let me know if you need pointers on that.
| apt-get install -y clang-11 && \ | ||
| apt-get install -y clang-tidy-11 && \ | ||
| apt-get clean && rm -rf /var/lib/apt/lists/ | ||
| RUN apt update \ |
There was a problem hiding this comment.
fyi: left a bunch of general items on my review comment for you to consider. thanks!
Thanks for the review, @anpaz, and for the kind words!
For both of these, I'd like to get @bromeg's and @efratshabtai's input as well, to make sure that aligns with experimental features more broadly.
Absolutely agreed. If it's alright, I'd like to do that after this PR completes, so that will allow for making independent PRs on each repo more easily.
Similarly here, I agree entirely with adding more test coverage. Ensuring that the noisy simulator agrees with the full-state simulator when configured to use the |
Yes, that would be the right set of test cases. In fact, if you take a look at any of the target package simulator tests, such as those built by Tests.Microsoft.Quantum.Simulators.Type1.csproj, you can see how they build just the subset of the tests that do gate validation. Something similar might work well for the opensim, though we may have to do some small work to generalize the constructor the test invokes. I agree that this can come in during a later PR to the feature branch before the merge to main. |
|
Sounds good, going to go on and merge this to the feature branch, then, with the following items in PRs to follow once microsoft/iqsharp#449 is merged:
|
|
The e2e build failed due to a known issue in the full-state simulator:
Given that e2e builds have been passing for a while, that local builds now work, and that this is to a feature branch, I'd like to go on and merge to unblock work on decompositions and test cases. Any objections, @bettinaheim, @anpaz, @swernli? |
|
Sounds good to me! |
* Add %simulate_noise and %noise_model magic commands. * Guard new magic behind experimental. * Fix to noise model source, use noise_model::ideal(). * Bump version and begin adding more documentation. * Add simulate_noise to qsharp-core. * Bump version and fix some syntax errors. * Add to noise model encoder. * Support setting opensim capacity through %config. * Started adding integration tests for experimental opensim. * Fixed tests. * Bump version and register display encoders. * Fix some display issues in noise model encoder. * fixing a bug in json noise model import (#412) * Update to 0.15.210324351-alpha. * Update to 0.15.210324357-alpha. * Capture more information in assertion logs. * Adapt to changes in noise model serialization API. * One more fix to serialization support. * Use alpha from microsoft/qsharp-runtime#709. * Support more processes and instruments in display encoders. * Allow Python client to inform kernel about experimental features. * Allow clients to override kernel name used to communicate with IQ#. * Fix serialization bug. * Allow loading noise models by name. * Revert accidental changes. * Allow getting/setting noise models by name, add docs. * Expose representations and named noise models. * Update package versions to latest alpha build. * Update to latest build. * Improve visualization and serialization. * Update to latest build. * Add Python magics, expose experimental build info to Python. * Display encoder for sparse stabilizer states. * Build info as a magic command. * Update to latest build. * Fix bug in group presentations. * Expose build info. * Fixed bug in mixed pauli process serialization. * Update to latest build. * One more slight encoder fix. * Move IQ# build to latest version of Ubuntu. * Trivial change to invalidate builds. * Use -any.whl to block Windows-specific wheels on Linux images. Co-authored-by: Sarah Kaiser <sckaiser@sckaiser.com>
* Start experimental simulators feature (#709) * Experiment to hook up Rust crate to cmake. * Add experimental simulator as rlib crate. * Expanded C API, started consolidating Cargo.toml. * Just the one crate will do. * First unit test of QIR ↔ opensim. * Add rust to builds. * Start adding to CI. * Don't set toolchain in CI. * Started building managed interface. * Add System.Text.Json dependency. * Add native opensim sharedlibs to nupkg. * Updated DLL name in NativeInterface. * downgrade s.t.json. * Fix pack.ps1 paths. * Set --release flag in opensim. * Add get_name. * Added rest of c_api to NativeInterface.cs. * Initial attempt at type1 implementation. * Add open systems data model. * Fix null reference exception by adding explicit qubit manager. * Started using QSharpCore interface temporarily. * Begin adding decompositions for opensim. * Expose noise_model::ideal() through c_api. * Fix json serialization of instruments. * More informative exceptions in SetNoiseModel * Pass DumpMachine through OnDisplayableDiagnostic. * Began addressing warnings and adding unit tests. * A couple more tests. * Started enabling doctests for Rust runtime. * Fix new doctest. * Added more API doc comments. * A bit of code cleanup. * Start adding integration tests for experimental simulator in C# runtime. * Fix teleport test. * Attempt at fixing project-based tests. * One more Rust-side unit test. * Added opensim to local dev props, and reverted bad change to unittests.qs. * Undo one other bad change. * Disable test that depends on Rz. * Started adding Rust-language integration tests. * Added more informative error message when R is not supported. * Forgot to add test data to prev commit. * Start enabling performance reporting for opensim. * Adapt opensim / managed interface to #538. * Adapt QIR / opensim interface to #553. * Apply rust-fmt, and reduce extraneous copying in conjugate_by. * Started weakening from owned arrays to views. * Weakened tensor to views by owning views and borrowing owns. * Started adding microbenchmarks. * extend_two_to_n * Clear diskspace and build microbenchmarks in CI. * Clean more aggressively to avoid disk space issues. * Format, fix warnings, simplify Cargo.toml. * Fix choco rustup in prereqs. * Disable choco rustup. * Fix path to Cargo.toml. * Don't check Rust-style names with clang-tidy. * M is no longer overriden. * Temporarily disable QIR / opensim integration. * Clean up extra cargo files after testing opensim. * Work on API docs and refactoring of Rust crate. * Separate out qdk_sim into its own crate. * Work on testing and enforcing rust fmt. * Run clippy in tests as well. * Add code quality checks to build/test. * Ignore jupyter temps and xplat drops. * Begin exposing build metadata through to C# callers. * Start injecting version number from build into Cargo.toml. * Started infrastructure to expose simulation functionality to Python. * Ignore Python temporaries and build artifacts. * More partial progress on exposing to Python. * Add initial precommit config. * Introduce fmt error. * Add new checks to pre-commit. * Continued integrating CHP into simulator API. * Address clippy warnings. * Work on docs. * Separate out paulis.rs. * Expose more simulation functionality to Python. * Generalize C API to allow using other state representations. * Enable CFG for C / Rust boundaries. * Move Cargo.toml injection into bootstrap. * Fix path from bootstrap to Cargo.toml injection, debug injection. * One more path fix. * One more path fix. * Remove --release from cargo package. * Revert CFG change to cmake. * Fix Rust build script. * Debug artifact paths. * Debug drop paths as well. * More path debugging. * Fix build paths. * Don't cargo clean build artifacts away. * Fix one character typo in Simulators.Dev.props. * Fix one more dev props typo. * Eliminate unneeded copy. * Improve naming conventions in processes.rs. * Fix pack.ps1 to include qdk_sim_rs drops. * Expose more information in exceptions. * Fix one more path. * Add links on cargo-clippy. * Fix issues in C API benchmarking. * Improve Python bindings for NoiseModel. * Improve readme for qdk_sim_rs crate. * Improve noise model handling, add test coverage. * Update System.Text.Json to 4.7.2. * Address clippy warnings. * Don't use new C# 9 features. * Don't store cargo lockfile in repo for now. * Remove outdated gitignore. * Fix serialization test, false negatives. * Use +nightly to run doctests. * Allow serializing and deserializing mixed pauli channels. * Expose as_json to more Python classes. * Added ideal_stabilizer noise model, split up processes, and start CHP decomp. * Allow applying CHP decompositions. * Fix clippy warnings. * Check error conditions on get_noise_model. * Allow for getting/setting noise models by name. * Expose as_json for State to Python. * Added stabilizer states to data model for serialization. * Significantly expanded documentation. * Added header explaining how to call cargo doc. * Fix doctest. * Add extra assertions. * Add more debug outputs to assertion failure. * Fixed assertion. * Fixed assert. * Fix clippy warning. * Update src/Simulation/Simulators/OpenSystemsSimulator/NativeInterface.cs Co-authored-by: Robin Kuzmin <kuzmin.robin@gmail.com> * Fixed C# serialization. * Slight comment fixes. * Fix build failure. * Address review comments. * qdk_sim → qdk_sim_experimental,improve Python version handling. * Revert src/Qir to main. QIR integration deferred to #713. * Add prerequisites.ps1. * Finish reverting QIR. * Generate C++ headers as well as C. * Fixed issue building docs locally with python disabled. * Updated link to tracking issue for experimental sims. * Gate `cargo clean` on CI builds. * Addressing feedback. * Force dependency on S.T.Encodings.Web to be >= 4.7.2. * Adding comment to explain serialization field. * Added README link to experimental simulators doc. * Fix typo. * Add stabilizers to state converter. * Move format/linting checks to build from test. * Fix stabilizer noise model serialization, added new test. * Work on docs. * Fixed some slight issues with n_qubits handling. * Update documentation notebook. * Regenerate notebook from latest build. * Fixed some serialization issues. * Fixed some serialization tests. * Expose build info and fix mixed pauli channels. * Updated documentation notebook from latest build. Co-authored-by: Robin Kuzmin <kuzmin.robin@gmail.com> * Finish remaining decompositions for initial version of experimental simulators. (#724) * Update documentation notebook for latest IQ# bugfix. * Started work on decompositions. * Added more detail to documentation notebook. * Moved all decomposition logic to Q# operations. * Added more decompositions. * Make known limitations more explicit in README.md. * Move known limitations to Markdown documentation./ * Started adding tests of decompositions. * Added missing `open` statement. * Don't mark decompositions as internal. * Fixed build issue (💕 to @swernli for all the help!) * Unit tests now pass. * Address feedback. * Fix two small build issues. * Added comment explaining reset / release. * Remove DLL added by accident. * Removed one more file added by accident. * Revert "Removed one more file added by accident." This reverts commit 1cc67a3. * Added comment explaining vcomp140.dll. * Call cbindgen manually (#730) * Call cbindgen manually instead of in build.rs. * Updated metadata in Cargo.toml. * Addressing feedback from @swernli. * Update src/Simulation/Simulators/OpenSystemsSimulator/README.md Co-authored-by: Robin Kuzmin <kuzmin.robin@gmail.com> * Update src/Simulation/qdk_sim_rs/docs/c-api.md Co-authored-by: Robin Kuzmin <kuzmin.robin@gmail.com> * Update src/Simulation/qdk_sim_rs/docs/c-api.md * Addressing feedback on c-api.md. Co-authored-by: Robin Kuzmin <kuzmin.robin@gmail.com>
* Add %simulate_noise and %noise_model magic commands. * Guard new magic behind experimental. * Fix to noise model source, use noise_model::ideal(). * Bump version and begin adding more documentation. * Add simulate_noise to qsharp-core. * Bump version and fix some syntax errors. * Add to noise model encoder. * Support setting opensim capacity through %config. * Started adding integration tests for experimental opensim. * Fixed tests. * Bump version and register display encoders. * Fix some display issues in noise model encoder. * fixing a bug in json noise model import (#412) * Update to 0.15.210324351-alpha. * Update to 0.15.210324357-alpha. * Capture more information in assertion logs. * Adapt to changes in noise model serialization API. * One more fix to serialization support. * Use alpha from microsoft/qsharp-runtime#709. * Support more processes and instruments in display encoders. * Allow Python client to inform kernel about experimental features. * Allow clients to override kernel name used to communicate with IQ#. * Fix serialization bug. * Allow loading noise models by name. * Revert accidental changes. * Allow getting/setting noise models by name, add docs. * Expose representations and named noise models. * Update package versions to latest alpha build. * Update to latest build. * Improve visualization and serialization. * Update to latest build. * Add Python magics, expose experimental build info to Python. * Display encoder for sparse stabilizer states. * Build info as a magic command. * Update to latest build. * Fix bug in group presentations. * Expose build info. * Fixed bug in mixed pauli process serialization. * Update to latest build. * One more slight encoder fix. * Move IQ# build to latest version of Ubuntu. * Trivial change to invalidate builds. * Use -any.whl to block Windows-specific wheels on Linux images. Co-authored-by: Sarah Kaiser <sckaiser@sckaiser.com> Co-authored-by: Sarah Kaiser <sckaiser@sckaiser.com>
This PR starts a new feature branch for experimental open systems and stabilizer / CHP simulator work. As this PR represents the start of that work, there's a fair bit that still needs done, but I wanted to open the PR for early initial feedback on the design of these experimental features.
The core functionality for the simulator is written as a Rust library with Python and native C APIs, along with a C# class to expose this library to Quantum Development Kit users. The Rust portions of the library are tested through unit and integration tests, and are checked against rustfmt and clippy for style and correctness. Performance of the Rust library is measured using criterion.rs, both at the C API boundary, and in microbenchmarks that exercise internal details; unit test and performance benchmarking results are integrated into CI.
TODO:
Controlled Hand other similar operations.ChpDecompositionvariant forqdk_sim::ProcessSee #504 for further discussion, and matching PR at microsoft/iqsharp#449.