Pretty-printing for Statevec and DensityMatrix (closes #501)#524
Pretty-printing for Statevec and DensityMatrix (closes #501)#524Vinny010 wants to merge 9 commits into
Conversation
…x#501) Add human-friendly rendering of quantum amplitudes and states: - `complex_to_str` in `pretty_print.py` recognizes common values and renders them exactly instead of as floats: fractions (`1/4`), square roots (`√2/2`) and complex exponentials (`e^(iπ/3)`). Recognition uses a square-then-rationalize heuristic and reuses the existing `angle_to_str` for the exponential phase. Supports ASCII, Unicode and LaTeX output. - `statevec_to_str` + `Statevec.draw` render a statevector in ket notation (e.g. `√2/2|00⟩ + √2/2|01⟩`), honouring the existing `encoding` parameter from `Statevec.to_dict`. - `density_matrix_to_str` + `DensityMatrix.draw` render a density matrix as a column-aligned grid (ASCII/Unicode) or a LaTeX `pmatrix`. - Tests covering the issue examples plus edge cases (zero, negative, pure imaginary, LaTeX, decimal fallback, symbolic), with numpy-style docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #524 +/- ##
==========================================
+ Coverage 88.85% 89.17% +0.31%
==========================================
Files 49 49
Lines 7135 7341 +206
==========================================
+ Hits 6340 6546 +206
Misses 795 795 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Cover the exponential-with-radius, cartesian, complex-decimal-fallback and LaTeX/ASCII imaginary branches of complex_to_str, the integer-times-root render path, and the negative-term, parenthesized-coefficient and unit-negative branches of Statevec.draw, addressing the patch-coverage gap reported on the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build the complex-amplitude statevec from a numpy array (numpy.complex128) rather than a Python complex literal: Python's complex only gained __complex__ in 3.11, so a bare complex is not a typing.SupportsComplex on 3.10 and is rejected by Statevec. Also use a real amplitude for the unit-negative case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thierry-martinez
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution!
Codecov has detected that some parts of your code are not tested.
As you may have noticed, PR #503 and PR #523 address the same issue (#501). We invite you to cross‑review each other's work: PR reviews are an inherent part of software development, just as important as writing code. Moreover, this review process will help you position your PR relative to the others, allowing us to determine which contribution best addresses the issue.
- Add a ``precision`` keyword argument to :func:`complex_to_str`, :func:`statevec_to_str`, :func:`density_matrix_to_str` and the matching :meth:`Statevec.draw` / :meth:`DensityMatrix.draw` methods to control the number of significant digits of the decimal fallback. The default value (``4``) preserves the previous behaviour, and a regression test exercises multiple precisions. - Rename ``_recognize_real`` to ``_recognize_sqrt`` to better reflect the ``signed_num * sqrt(inner) / den`` form the helper returns (pure rationals are covered as ``inner == 1``); update the docstring accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review, @thierry-martinez! Pushed eebb104 addressing both inline points:
I'll have a look at #503 and #523 and follow up with comments there. |
|
@thierry-martinez — small process note: looks like the workflow run on |
thierry-martinez
left a comment
There was a problem hiding this comment.
Thank you for your update, @Vinny010. I think we are on a good track to merge this PR. In this respect, please add a comment in the discussion of #501 so that I can assign the issue to you.
I have also made a few remarks (as a side note, I did not use the "Request changes" flag since we've chosen not to use it in the Graphix project, to avoid blocking the PR from being merged if other maintainers approve it). Note that we have a two‑approval rule for non‑trivial PRs: therefore, once I or another maintainer approves this PR, we'll need a second approval before merging.
- Use \ket{} in LaTeX kets instead of \lvert ... \rangle
- Render pure imaginary values with the unit leading the numerator
(i/2, -i√2/2) via a dedicated _render_imaginary helper
- Prefer the Cartesian form over a radius-prefixed exponential when
|z| != 1 (1 + i instead of √2·e^(iπ/4)); keep the radius-prefixed
exponential only as a last resort before the decimal fallback
- Merge the format-specific complex_to_str tests into a single
parametrized test keyed by a Mapping[OutputFormat, str]
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thierry-martinez
left a comment
There was a problem hiding this comment.
Thank you for your changes. Just minor comments left. Please have a look at the test coverage: there are still some gaps.
Responds to @thierry-martinez's review on TeamGraphix#524: - Add an `rtol` parameter everywhere an `atol` is exposed: `complex_to_str`, `density_matrix_to_str`/`DensityMatrix.draw` (it was missing), and threaded through the recognition helpers into the `math.isclose` calls. The default (0.0) preserves the previous behaviour. - `_decimal_to_str` now takes `atol` as an argument instead of relying on the module-level `_DEFAULT_ATOL`. - Test coverage: - Merge the remaining `complex_to_str` value tests (radius/Cartesian/decimal fallback/integer-times-surd) into the parametrized `test_complex_to_str_values`. - Cover the LaTeX `\ket{...}` notation in the statevec draw tests (was untested). - Add tests exercising `rtol` (recognition control + `DensityMatrix.draw`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @thierry-martinez — all four addressed in 58eb451.
|
thierry-martinez
left a comment
There was a problem hiding this comment.
I extracted the coverage results from Codecov. You can view them here: https://app.codecov.io/gh/TeamGraphix/graphix/pull/524?src=pr&el=tree
| encoding: _ENCODING = "MSB", | ||
| max_denominator: int = _DEFAULT_MAX_DENOMINATOR, | ||
| atol: float = _DEFAULT_ATOL, | ||
| rtol: float = 0.0, |
There was a problem hiding this comment.
| rtol: float = 0.0, | |
| rtol: float = _DEFAULT_RTOL, |
Responds to @thierry-martinez's follow-up review on TeamGraphix#524: - Factor a magnitude shared by every amplitude in the ket-notation output, so a state prints as `√2/2(|00⟩ + |11⟩)` rather than `√2/2|00⟩ + √2/2|11⟩` (signs are kept inside the parentheses). Compound (Cartesian) and non-uniform amplitudes fall back to the previous term-by-term rendering. Updated the affected tests and the docstring / doctest examples. - Cover the branches flagged as untested: the tiny-real -> "0" path (square-free decomposition and real-renderer zero branches), the decimal imaginary fallback, the exponential form with an unrecognized radius, and the empty-statevector "0". - Drop the redundant `x == 0` guard in `_recognize_sqrt`: the general path already returns `(0, 1, 1)` for `x == 0`, so the early return was dead code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @thierry-martinez — follow-up addressed in 5b7c389. Uniform magnitude. Statevec output now factors a magnitude shared by every amplitude: Uncovered lines. Added tests reaching each flagged branch:
For the (I checked the new branches execute by tracing the outputs; |
|
Note on the red CI here: the 7 failures are all in Those render graph/flow figures and don't touch anything in this diff, which only changes amplitude/statevector text formatting — nothing in Happy to help if a baseline refresh is wanted — if you let me know the matplotlib version the CI pins to, I can regenerate the references against it; or if you'd rather keep baseline changes out of this PR, that's fine too and I'll leave it to you. |
|
Yes, indeed, the CI broke because of a new Matplotlib release. It was fixed in #541, which has now been merged. Could you merge the current |
As I mentioned in my previous comment, you can view the code‑coverage results here: https://app.codecov.io/gh/TeamGraphix/graphix/pull/524 |
The uniform-magnitude factoring moved the negative-amplitude case off the
term-by-term loop, leaving `result += f" - {term[1:]}"` (pretty_print.py:847)
uncovered. Give `test_statevec_draw_non_uniform_not_factored` a negative second
amplitude so it flows through the fallback and exercises the ` - ` separator.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the codecov link — that pinned it to a single line. The patch coverage was 99.5%, with one miss: It was a side effect of the new uniform-magnitude factoring: the negative-amplitude case I had a test for ( |
pranav97nair
left a comment
There was a problem hiding this comment.
Hi @Vinny010 , thanks for your work on this PR! This will be a nice addition to Graphix. I have a couple points to add.
Currently, the statevector draw function factors out the magnitude when it is shared by all component amplitudes. I would however like to see this factoring even in the case when there is a relative phase, as this does not change the magnitude of the affected component. A basic example:
plus = Statevec(data=BasicStates.PLUS)
assert plus.draw() == "√2/2(|0⟩ + |1⟩)" # Passes
plus_i = Statevec(data=BasicStates.PLUS_I)
assert plus_i.draw() == "√2/2(|0⟩ + i|1⟩)" # FailsCould you update your factoring function to account for this case and enforce factoring when all magnitudes are equal up to a relative phase
Also, could you include a test checking the impact of the max_denominator parameter of the new draw() functions? As it stands, I believe none of the tests use anything other than the default value.
…enominator test - Factor the shared modulus even when components differ by a relative phase, so the statevector draw renders e.g. `√2/2(|0⟩ + i|1⟩)` (PLUS_I) and `√2/2(|0⟩ - i|1⟩)` (MINUS_I), not just the all-equal-coefficient case. `_factor_uniform_magnitude` now works on the amplitudes: it factors a common modulus `r` and keeps each per-component phase `amplitude / r` inside the parentheses. Falls back to term-by-term when the moduli differ, the modulus is trivial (1), or the amplitudes are non-numeric (symbolic). - Add tests for the relative-phase factoring (PLUS / PLUS_I / MINUS_I, Unicode + LaTeX) and for the `max_denominator` parameter of `DensityMatrix.draw` / `Statevec.draw` (1/2 -> 0.5 and √2/2 -> 0.7071 when capped), plus direct edge-case coverage of the factoring helper (non-numeric, zero modulus, unit modulus). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @pranav97nair — both addressed in 0faf587. Relative-phase factoring. Statevec(data=BasicStates.PLUS).draw() # √2/2(|0⟩ + |1⟩)
Statevec(data=BasicStates.PLUS_I).draw() # √2/2(|0⟩ + i|1⟩)
Statevec(data=BasicStates.MINUS_I).draw() # √2/2(|0⟩ - i|1⟩)It still falls back to term-by-term when the moduli genuinely differ, when the shared modulus is trivial (
Local run is green (ruff / mypy / 185 tests + doctests). Let me know if you'd like the relative phase rendered differently (e.g. an explicit |
Closes #501.
Adds human-friendly rendering of amplitudes and states:
complex_to_str— renders common values exactly (1/4,√2/2,e^(iπ/3)) via a square-then-rationalize heuristic, reusing the existingangle_to_strfor the exponential phase. Supports ASCII / Unicode / LaTeX.statevec_to_str+Statevec.draw— ket notation, honouring theto_dictencodingparameter (e.g.√2/2|00⟩ + √2/2|01⟩).density_matrix_to_str+DensityMatrix.draw— column-aligned grid (ASCII/Unicode) or LaTeXpmatrix.Passes
ruff check,ruff format,mypy --strict, andpytestlocally (167 tests in the affected suites).Design points I'd value feedback on:
draw()returns astr, consistent with the existingto_unicode/to_latexstyle.Happy to adjust either.
Developed with LLM assistance, reviewed and tested by me.