QCDL: validate the arguments and signature of the qcdl decorator - #75
QCDL: validate the arguments and signature of the qcdl decorator#75qci-amos wants to merge 1 commit into
Conversation
The decorator injects qubits into the decorated function by name, so a parameter that is not a q<N> silently received nothing and python's own error named only the parameter, with no hint that qubits were involved. A generated qubit with no parameter to land in was dropped from the program without a word. Both are now reported: the unfilled-parameter TypeError keeps python's wording and then explains the q<N> rule and what was supplied, and a qubit with nowhere to go raises QCDLUserError. Signatures taking **kwargs absorb everything, and an environment is still allowed to supply more qubits than the signature names, since it always supplies its whole set. num_qubits is also checked up front, including a bare @qcdl, which otherwise passes the decorated function in as the qubit count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
==========================================
+ Coverage 90.18% 90.23% +0.05%
==========================================
Files 31 31
Lines 5317 5447 +130
==========================================
+ Hits 4795 4915 +120
- Misses 522 532 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| f" {getattr(num_qubits, '__name__', num_qubits)} with @qcdl() or" | ||
| f" @qcdl(num_qubits) rather than with a bare @qcdl" | ||
| ) | ||
| if isinstance(num_qubits, bool) or not isinstance(num_qubits, numbers.Integral): |
There was a problem hiding this comment.
Curious what the reason is to not do: type(num_qubits) is not int
| parameter silently receives nothing. Python's own message for that names | ||
| only the parameter, which gives no hint that qubits are involved. | ||
| """ | ||
| plural = "" if len(missing) == 1 else "s" |
There was a problem hiding this comment.
This seems a big code investment compared to required positional argument(s) for line 631. Is it worth complicating the code for such a thing?
There was a problem hiding this comment.
My viewpoint on this sort of consideration has changed dramatically the last several months. Last year I would have agreed with you with a passion, but now... if ai is "willing" to do the grunt work, and back it up with tests, then why not just get the best error message possible? Furthermore, in my experience occasionally subtle changes in wording can help someone providing support narrow down what went wrong.
|
|
||
| if any(is_qubit_or_coupler_name(name) for name in missing): | ||
| message += ( | ||
| " Raise num_qubits to cover the missing qubits, or drop the" |
There was a problem hiding this comment.
Just checking: is this correct? Haven't followed it carefully down the stack but if sufficient qubits are not named should the qubit number not be lowered?
| " parameters for them." | ||
| ) | ||
| else: | ||
| matches = "does not match" if len(missing) == 1 else "do not match" |
There was a problem hiding this comment.
Same comment: for simpler code " not a match for that pattern" seems good enough for an eror message. Also, for the plural used here and taken from above
| f" {', '.join(module_kwargs)} but" | ||
| f" {getattr(f, '__name__', 'the decorated function')}()" | ||
| f" has no parameter for {', '.join(dropped)}, so" | ||
| f" {'they' if len(dropped) > 1 else 'it'} would be" |
There was a problem hiding this comment.
| f" {'they' if len(dropped) > 1 else 'it'} would be" | |
| f" would be" |
If accepted might be easier to do manually to keep a good line length
2 participants
Part of splitting #71 into reviewable pieces. Independent of the other four.
The problem
The
qcdldecorator injects qubits into the decorated function by name, askeyword arguments
q0,q1, ... Three things went wrong quietly:q<N>receives nothing. Python's own message —main() missing 1 required positional argument: 'alpha'— gives no hint thatqubits are involved or that the name is the reason.
without a word:
@qcdl(3)ondef main(q0, q1)built a two-qubit program.num_qubitswas never checked.@qcdl(0),@qcdl(2.5)and a bare@qcdl(which passes the decorated function in as the qubit count) all failed later,
obscurely or not at all.
The change
_validate_num_qubitschecks the count up front, including the bare-@qcdlcase, which gets its own message telling you to call the decorator.
At call time, any required parameter the call would leave unbound is reported
with a
TypeErrorthat keeps Python's own wording and then explains the rule:which qubits were supplied, that they are injected as
q<N>, and whether thefix is to raise
num_qubitsor to rename the parameter. Separately, agenerated qubit with no parameter for it raises
QCDLUserErrorrather thanbeing dropped.
Two cases are deliberately left alone:
**kwargsabsorbs every qubit, so nothing is dropped;naming fewer is expected and still allowed.
Compatibility
Programs that were silently losing a qubit, or that would have raised a
confusing
TypeErroranyway, now fail with an explanation. Everything thatbuilt the program the author intended is unchanged.
Testing
pytest tests/passes.tests/test_qcdl_circuit.pygains two classes coveringnum_qubitsvalidation and each signature shape: dropped qubit,**kwargs,environment mode, non-
q<N>parameter, extra qubit parameter, several unfilledparameters at once, keyword-only parameters, caller-supplied values, and
inferred mode with a sparse set (
q0,q5).🤖 Generated with Claude Code