Implement sparse isometry with binary encoding#435
Open
RushiGong wants to merge 26 commits into
Open
Conversation
📊 Coverage Summary
Detailed Coverage ReportsC++ Coverage DetailsPython Coverage DetailsPybind11 Coverage Details |
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 7, 2026
YingrongChen
reviewed
Apr 8, 2026
RushiGong
requested review from
ConradJohnston,
nabbelbabbel,
sobolevnrm and
wavefunction91
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 22 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <copilot@github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 22 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
python/src/qdk_chemistry/utils/qsharp/src/StatePreparation.qs:12
- These imports appear unused in this file (
ApplyControlledOnBitString,MResetZ, andStd.TableLookup.Select). Unused imports add noise and can trigger compiler/linter warnings; consider removing them or using them if intended.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+100
to
+102
| Logger.info( | ||
| f"Binary encoding produced {len(params['binaryEncodingOps'])} operations " | ||
| f"for {n_qubits}-qubit system with {len(bitstrings)} determinants " |
There was a problem hiding this comment.
This log message references n_qubits and bitstrings, but neither is defined in _run_impl (they’re locals inside _build_binary_encoding_params). This will raise NameError on every run. Use values derived from params (e.g., params['numQubits'] and len(params['stateVector']) / determinant count) or compute these in _run_impl before logging.
Suggested change
| Logger.info( | |
| f"Binary encoding produced {len(params['binaryEncodingOps'])} operations " | |
| f"for {n_qubits}-qubit system with {len(bitstrings)} determinants " | |
| n_qubits = params["numQubits"] | |
| determinants = params.get("bitstrings", params.get("stateVector", [])) | |
| Logger.info( | |
| f"Binary encoding produced {len(params['binaryEncodingOps'])} operations " | |
| f"for {n_qubits}-qubit system with {len(determinants)} determinants " |
| import Std.Math.Ceiling; | ||
| import Std.Math.Lg; | ||
| import Std.Measurement.MResetX; | ||
| import Std.StatePreparation.PreparePureStateD; |
There was a problem hiding this comment.
PreparePureStateD is imported here but not referenced in this file (dense prep is delegated to ApplyDensePreparation). Consider removing the unused import to avoid warnings and keep dependencies minimal.
Suggested change
| import Std.StatePreparation.PreparePureStateD; |
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <copilot@github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 22 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
python/src/qdk_chemistry/utils/qsharp/src/StatePreparation.qs:13
- These
importstatements appear to be unused in this file (Std.Canon.ApplyControlledOnBitString,Std.Measurement.MResetZ, andStd.TableLookup.Select). If they’re no longer needed after the refactor toApplyMatrixCompressionOp, consider removing them to avoid Q# compiler warnings and keep dependencies minimal.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| program=QSHARP_UTILS.BinaryEncoding.MakeBinaryEncodingStatePreparationCircuit, | ||
| parameter=params, | ||
| ) | ||
| qsharp_op = QSHARP_UTILS.BinaryEncoding.MakeBinaryEncodingStatePreparationOp(*params.values()) |
📊 Coverage Summary
Detailed Coverage ReportsC++ Coverage DetailsPython Coverage DetailsPybind11 Coverage Details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.