Circumvent constructing unregistered program type nodes in ConversionGraph#986
Circumvent constructing unregistered program type nodes in ConversionGraph#986ryanhill1 merged 6 commits intoqBraid:mainfrom
ConversionGraph#986Conversation
ConversionGraph
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
|
Hi @ryanhill1, I observe that when doing the following, unregister_program_type("qasm2")
new_graph = ConversionGraph()
print(new_graph.has_node("qasm2")) # Shows False, should be False.
new_graph_2 = ConversionGraph(nodes=["qasm2"])
print(new_graph_2.has_node("qasm2")) # Shows True (Is this correct?).Could you confirm if this behavior is correct? If not, I think the issue potentially stems from not initiating |
There was a problem hiding this comment.
These changes look good so far!
Could you add a test case to test_conversion_graph.py that verifies your solution? e.g. that creates a conversion graph, checks that a given node is present, unregisters the corresponding program type, creates a new graph, and then checks that the node is no longer included?
Also, you raised a good point when you referenced:
new_graph_2 = ConversionGraph(nodes=["qasm2"])
print(new_graph_2.has_node("qasm2")) # Shows True (Is this correct?).
I don't think one should be allowed to specify a node to use in a ConversionGraph that corresponds to a program type that isn't registered. So ideally, I think that first line should raise an exception (maybe a PackageValueError?). Although, this might require updates to existing tests in test_conversion_graph.py, which could be a bit of a hassle. If those updates end up being too involved, we can always spin this off as a separate issue—just let me know.
|
Okay, I am unsure why the test fails for py3.13. The tests I am running locally are on py3.10 and are successful. However, when bumping to py3.13, I find that QPROGRAM_REGISTRY = {'openqasm3': <class 'openqasm3.ast.Program'>, 'qasm3': <class 'str'>, 'qasm2': <class 'str'>, 'qasm2_kirin': <class 'str'>, 'qubo': <class 'qbraid.programs.typer.QuboCoefficientsDict'>, 'ionq': <class 'qbraid.programs.typer.IonQDict'>}Is this because these packages are not yet compatible with 3.13? Shall I skip the unit test for py3.13? @ryanhill1 I can do the following, @pytest.mark.skipif(sys.version_info[:2] == (3,13), reason="Some program_types are not available for python3.13.")
@pytest.mark.parametrize("program_type", ["qasm2", "qasm3", "pyquil", "braket", "cirq"])
def test_unregistered_node_in_conversion_graph(program_type):
"""Test the unregistered nodes in ConversionGraph"""
assert program_type in QPROGRAM_ALIASES |
Summary of changes
Okay, so I could figure out that the main error comes from loading the default conversions in this line,
qBraid/qbraid/transpiler/graph.py
Lines 132 to 147 in d3f6614
This PR makes changes to only consider
sourceandtargetinconversion_functionsthat are present/registered inQPROGRAM_ALIASES, hence solving the bug mentioned in #789.Closes #789.