Qiskit / qiskit-qasm3-import

Importer from OpenQASM 3 to Qiskit's QuantumCircuit
https://qiskit.github.io/qiskit-qasm3-import
Apache License 2.0
15 stars 7 forks source link

Add support for `!` on scalar `bit` types #17

Closed jakelishman closed 1 year ago

jakelishman commented 1 year ago

It's not 100% clear from the OQ3 spec whether this is permitted, but Qiskit certainly outputs using this, the IBM QE stack allows this, and it generally sensible.

Fix #14

jlapeyre commented 1 year ago

After reading the OQ3 spec, it seems clear to me that in the constructions $$\text{if} ( expr \text{)} \quad \text{ and } \quad \text{while} ( expr \text{)} $$ the expression expr must evaluate to a bool.

I don't see any support for truthiness in the spec. I guess that while (!_bit0) should instead be while (_bit0 == 0)

jakelishman commented 1 year ago

I agree there's insufficient rigour in the spec around these types, but in practice this is currently what Qiskit outputs and IBM QE accepts, and at worst it's giving some unambiguous meaning to OQ3 programs that were previously invalid. I think it's not entirely clear what (if any) the difference between a bare bit (as opposed to a bit[n]) and a bool is meant to be in the OQ3 spec (if there is one at all); they're both two-valued types whose values represent the concepts of "set" and "not set", and I'm not certain that there's any meaningful semantic differences between the two that would motivate them being separate types. (Of course bit[n] is a different beast, but this is just about the scalar type bit.)

For now, I think this package should probably at least read in these conditions in the same form that Qiskit puts them out (for non-Expr nodes), and I think it might be better to tighten the semantics on both sides if more clarification in the OQ3 spec / the platforms we know use our outputs change.

jlapeyre commented 1 year ago

The spec is not particularly unclear on the matter. It says that bit and bool are different types, and the conditionals must use the latter. I guess there is no reason to have separate bit and bool types. It's probably worth doing a survey (if we haven't already done it) and clearing this up.

Maybe: bit[n] represents an array of bits. But when dealing with individual bits, you always work with type bool. That is

bit[3] bit_array = "111";
bool x; 
\\ bit x;    illegal
x = bit[3];

The languages I can think of off the top of my head do this or something similar. Like just use bool for everything. Julia has both bit arrays and an array of bools. But the elements of both are always semantically bools.

I'm concerned about supporting something that's not clean, but becomes common and has to be supported in the future. I think while(x != 0) and so forth is not elegant. But keeping both bit and bool and giving bit some of bools semantics is redundant and complex.

What I really would not like to see is int[16] x;, while(x) and so forth.

jakelishman commented 1 year ago

I'd like to get this package aligned with both Qiskit and QE in terms of what it accepts (which this PR does), and if what exactly that behaviour is should change, then do that in a more unified manner once we're sure what that should be.

Fwiw, this PR does not newly cause bit to be accepted as the type of an if - this package has always allowed that, in line with the QE stack - the new behaviour is effectively a bit -> bool overload for the ! unary operator.