Fix histogram() for qudits by adding fold_base parameter#8138
Conversation
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
Please rewrite the PR description in style of commit messages of the main branch using your own words and thinking. The description should be brief, to the point and without redundant AI checklists.
| key: TMeasurementKey, | ||
| fold_func: Callable[[tuple], T] = cast(Callable[[tuple], T], value.big_endian_bits_to_int), | ||
| fold_func: Callable[[tuple], T] | None = None, | ||
| qid_shape: tuple[int, ...] | None = None, |
There was a problem hiding this comment.
I suggest to rename the new argument to fold_base with the same possible values as in big_endian_digits_to_int. This would make it more convenient to handle same-dimension qudits as their dimension would not need to be repeated.
There was a problem hiding this comment.
Renamed to fold_base. Fold_base=3 is much cleaner than repeating the shape.
| qid_shape: The qudit dimensions for the measured qubits. If | ||
| specified, the default fold_func will use this to correctly | ||
| interpret measurement results for qudits with dimension > 2. | ||
| If not specified, defaults to base-2 (qubit) interpretation. |
There was a problem hiding this comment.
The second line needs to be indented as in the fold_func doc above.
I suggest to raise ValueError when both the fold_func and the qid_shape / fold_base are specified. The docstring should make it clear that the new argument is a convenience way of providing fold_func for qudits.
| ) | ||
| assert result.histogram(key='q', qid_shape=(3, 3, 3)) == collections.Counter({26: 1}) | ||
| # Verify backward compatibility: without qid_shape, base-2 is used | ||
| assert result.histogram(key='q') == collections.Counter({7: 1}) |
There was a problem hiding this comment.
This is incorrect, the default fold_func for qubits should raise an error if there is anything else than 0 or 1 in the measured array.
If you can do the work without outsourcing to an LLM, please update big_endian_bits_to_int to raise an error for the above. If this would be just passed on to an AI tool, please delete the assertion and let it be.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8138 +/- ##
==========================================
- Coverage 99.60% 99.60% -0.01%
==========================================
Files 1118 1118
Lines 101043 101054 +11
==========================================
+ Hits 100643 100653 +10
- Misses 400 401 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Su760
changed the title
2 participants
The histogram function uses base-2 by default, which gives incorrect results for qudits.
For example, three qutrits, each measuring 2, give 7 instead of 26.
I added an optional fold_base parameter to specify the qudit dimension. Raises ValueError if both fold_func and fold_base are passed.
Fixes #5926