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

Some cases of wrong QASM code return empty exception #24

Open matrixik opened 10 months ago

matrixik commented 10 months ago

In some cases when providing wrong QASM code exceptions are empty

python --version

Python 3.10.12

[tool.poetry.dependencies]

qiskit-qasm3-import = "^0.4.1"

Tests example:

from qiskit.qasm3 import loads

test_cases = [
    ('OPENQASM 3.0; include "stdgates.inc";'),
    ('OPENQASM 3.0;include "stdgates.inc";qubit[2] q;h q[0];cx q[0], q[1];'),
    ("INVALID QASM STRING"),
    ("kjkasdhgfjkgh"),
    ("const int size = 4;"),
    ("int size = 4;"),
    ("float[64] ang = pi/2;"),
    # Add more test cases here...
]

for test in test_cases:
    try:
        print(f"\nSuccessful case: {test}\nOutput:\n{loads(test)}\n")
    except Exception as e:
        print(f"Failed case: {test}, Exception: {e}")

Output:

 % python short_qiskit_tests.py

Successful case: OPENQASM 3.0; include "stdgates.inc";
Output:

Successful case: OPENQASM 3.0;include "stdgates.inc";qubit[2] q;h q[0];cx q[0], q[1];
Output:
     ┌───┐
q_0: ┤ H ├──■──
     └───┘┌─┴─┐
q_1: ─────┤ X ├
          └───┘

Failed case: INVALID QASM STRING, Exception:
line 1:13 no viable alternative at input 'kjkasdhgfjkgh'
Failed case: kjkasdhgfjkgh, Exception:
Failed case: const int size = 4;, Exception: '1,0: node of type ConstantDeclaration is not supported'
Failed case: int size = 4;, Exception: "1,0: declarations of type 'int' are not supported"
Failed case: float[64] ang = pi/2;, Exception: "1,0: declarations of type 'float' are not supported"
matrixik commented 10 months ago

pytest format:

import pytest
from qiskit.qasm3 import loads

test_cases = [
    ('OPENQASM 3.0; include "stdgates.inc";', False),
    ('OPENQASM 3.0;include "stdgates.inc";qubit[2] q;h q[0];cx q[0], q[1];', False),
    ("INVALID QASM STRING", True),
    ("kjkasdhgfjkgh", True),
    ("const int size = 4;", True),
    ("int size = 4;", True),
    ("float[64] ang = pi/2;", True),
    # Add more test cases here...
]

@pytest.mark.parametrize("input_string,should_error", test_cases)
def test_loads(input_string, should_error):
    if should_error:
        with pytest.raises(Exception):
            loads(input_string)
    else:
        try:
            loads(input_string)
        except Exception:
            pytest.fail(
                f"loads() raised Exception unexpectedly for input: {input_string}"
            )
jakelishman commented 10 months ago

The times you're seeing an empty message are when the underlying ANTLR parser is failing, and the cases that have messages are when the conversion to Qiskit that is actually part of this package is throwing the exception.

We can potentially put a generic message in the case that ANTLR fails, but largely this package is a fairly thin wrapper around the reference parser at github.com/openqasm/openqasm, so there's a limited amount we can do from here.

matrixik commented 10 months ago

Thank you for your reply.

From my perspective exception should always contain information what happen, even if it's only something generic that hint us where to search for exception source. I also don't like when library is outputting anything to stdout or stderr (pytest is not showing this msgs by default). This stuff should always be returned as part of the exception message.

jakelishman commented 10 months ago

I agree with you - this is actually one of the reasons we're moving away from this thin wrapper package to replace it with a more in-house OpenQASM 3 parser: so we can provide better error messages (see #13). The ANTLR-based parser isn't entirely within our control, and even if it were, ANTLR's Python runtime doesn't offer the best developer tooling for writing good diagnostics and improving the failure paths.