Repository navigation
Use Trim SQD in SQD notebooks - #5834
Kevin J. Sung (kevinsung) wants to merge 6 commits into
Conversation
|
One or more of the following people are relevant to this code:
|
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Henry Zou (henryzou50)
left a comment
There was a problem hiding this comment.
Thanks, Kevin, this is a nice update and looks good to me! I was able to cleanly run both notebooks and reproduce the simulator results. I didn't have the chance to do the hardware runs, but the new results look better and the sections overall look good as well.
Here are some comments and minor suggestions I had:
Comments
1. Both notebooks: "CI string". The new prose uses "CI string" without defining it. SQD step 4 says trimming keeps "configurations", but it actually ranks per-spin CI strings, and the merged subspace is their product. A one-line definition at first use, plus "CI strings" in step 4, would make the trim description accurate and easier to follow.
2. SKQD, small simulator example. The new StatevectorSampler(seed=rng) doesn't make the results reproducible, because GenericBackendV2 and the preset pass manager are unseeded and the layout changes between runs. Three local reruns gave merged dims of 484–625, 2 or 5 iterations, and errors of 1e-4 to 4e-4. Adding seed= to GenericBackendV2 and seed_transpiler= fixed it for me. The SQD notebook has the same unseeded backend.
3. Links (depends on #5855). The docs site's copy of the SQD API reference is still 0.13.1. As a result, the SQD tutorial's "API documentation" link opens a page with no policy argument and no TrimPolicy. #5855 updates it to 0.14 and adds a trim SQD guide page. Once that merges, could you switch the trim-guide links in both notebooks from github.io to /docs/addons/qiskit-addon-sqd/guides/trim-sqd, and link TrimPolicy to /docs/api/qiskit-addon-sqd/trim#trimpolicy (in this PR or a follow-up)?
Minor suggestions
- SQD, Step 4 paragraph before the simulator SQD run. Change "to keep the merged subspace from becoming smaller than the batches" to "to keep the merged subspace from becoming much smaller than the batches." With
max_dim=50, the merged subspace can never be larger than a batch (here 40–41 CI strings per sector versus 50), so the high ratio only limits how much smaller it gets. - SQD, hardware bullet for
trim_ratio. Replace the bullet with: "Lowertrim_ratioto0.5. This system has far more possible CI strings than fit in a subspace, so with two batches, a ratio of0.5keeps the merged subspace no larger than a single batch." The current "few CI strings in common" doesn't hold after iteration 1, because carryover strings go into every batch. - Both notebooks, the paragraph that introduces
TrimPolicy. After the sentence about three diagonalizations per iteration, add: "The callback receives only the merged result, so each iteration prints a single energy." Otherwise readers may wonder why three diagonalizations produce one line of output.
Description
Also updates the SQD parameters and plots in the SKQD notebook.
Related issues
Fix #5799
Fix #5762
Checklist