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

Using circuit.bits as args to add_wasm with more registers causes a Fatal Python error. #1641

Closed
johnchildren opened this issue Oct 29, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@johnchildren
Copy link
Contributor

Using a test case like this in test_classical.py

def test_wasm_circuit_bits() -> None:
    w = wasm.WasmFileHandler("testfile.wasm")
    c = Circuit(0, 6)
    c.add_wasm("add_one", w, [1], [1], c.bits)
    assert c.depth() == 1

Results in an error during the tests of:

classical_test.py ................[2024-10-29 15:13:09] [tket] [critical] Assertion 'sum_of_i32 == n_' (/build/tket-sources/src/Ops/ClassicalOps.cpp : WASMOp : 266) failed>
Fatal Python error: Aborted

Current thread 0x00007ffff7f8f740 (most recent call first):
  File "/nix/store/sldmq9pa6aw0plb54q6lc9maydx6a35k-python3.12-pytket-1.34.0/lib/python3.12/site-packages/pytket/circuit/__init__.py", line 103 in overload_add_wasm
  File "/build/test_root/tests/classical_test.py", line 363 in test_wasm_circuit_bits

Not that reducing the number of classical registers in the circuit or using a different args avoids this issue.

@cqc-alec cqc-alec added the bug Something isn't working label Oct 29, 2024
@johnchildren
Copy link
Contributor Author

You can also trigger this bug with:

def test_wasm_circuit_bits() -> None:
    w = wasm.WasmFileHandler("testfile.wasm")
    c = Circuit(0, 1)
    c.add_wasm("add_one", w, [1], [1], [0])
    assert c.depth() == 1

@cqc-melf cqc-melf self-assigned this Nov 5, 2024
@cqc-alec
Copy link
Collaborator

cqc-alec commented Nov 6, 2024

I noticed recently that the unit ID we use for the wasm state (which has the special name _w) has type Bit, which seems wrong. Could this be related @cqc-melf ?

@cqc-melf
Copy link
Contributor

cqc-melf commented Nov 7, 2024

@cqc-alec I have taken a look on this, it should have the type WasmState, see

class WasmState : public UnitID {

Can you give some more details where you have seen this? I was not able to construct an example where this did not work.

@cqc-alec
Copy link
Collaborator

cqc-alec commented Nov 7, 2024

@cqc-alec I have taken a look on this, it should have the type WasmState, see

tket/tket/include/tket/Utils/UnitID.hpp

Line 229 in a9add4b
class WasmState : public UnitID {

Can you give some more details where you have seen this? I was not able to construct an example where this did not work.

In this test, if you insert the lines

    cmd = c.get_commands()[0]
    for arg in cmd.args:
        print(arg, type(arg))

after constructing the circuit, and run it with pytest -s, you see:

c[0] <class 'pytket._tket.unit_id.Bit'>
c[1] <class 'pytket._tket.unit_id.Bit'>
_w[0] <class 'pytket._tket.unit_id.Bit'>

(This was the reason for this workaround.)

@johnchildren
Copy link
Contributor Author

Is the issue that there is no python binding for the WasmState class? Should it even have a python binding?

@cqc-melf
Copy link
Contributor

cqc-melf commented Nov 7, 2024

Is the issue that there is no python binding for the WasmState class? Should it even have a python binding?

Probably yes, still unexpected that they show up as bits. I will add them.

@cqc-melf
Copy link
Contributor

cqc-melf commented Nov 7, 2024

@cqc-alec I have taken a look on this, it should have the type WasmState, see
tket/tket/include/tket/Utils/UnitID.hpp
Line 229 in a9add4b
class WasmState : public UnitID {
Can you give some more details where you have seen this? I was not able to construct an example where this did not work.

In this test, if you insert the lines

cmd = c.get_commands()[0]
for arg in cmd.args:
    print(arg, type(arg))

after constructing the circuit, and run it with pytest -s, you see:

c[0] <class 'pytket._tket.unit_id.Bit'>
c[1] <class 'pytket._tket.unit_id.Bit'>
_w[0] <class 'pytket._tket.unit_id.Bit'>

(This was the reason for this workaround.)

Thank you! I will solves this together with the other issues.

@cqc-melf cqc-melf mentioned this issue Nov 21, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants