Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cases of wrong QASM code return empty exception #24

Open
matrixik opened this issue Nov 16, 2023 · 4 comments
Open

Some cases of wrong QASM code return empty exception #24

matrixik opened this issue Nov 16, 2023 · 4 comments

Comments

@matrixik
Copy link

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"
  • In case of INVALID QASM STRING exception is empty
  • In case of random string (like kjkasdhgfjkgh) exception is empty but additionally there is line printed that is not included in exception: line 1:13 no viable alternative at input 'kjkasdhgfjkgh'
@matrixik
Copy link
Author

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
Copy link
Member

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
Copy link
Author

matrixik commented Nov 17, 2023

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants