Tuple argument in __call__ causes fault error

[jack-melchert]

I've added a test case to test_magma.py on fault_tuple_bug that uses a call function like this:

Bit = family.Bit
input_t = Tuple[Bit, Bit]

@family.assemble(locals(), globals())
class PE_Enum(Peak):
     def __call__(self, op: Op, inputs: input_t) -> Bit:

There seem to be multiple options for how to assign a value to "inputs" in fault.

input_t = Tuple[Bit, Bit]
PE_magma = PE_fc(family.MagmaFamily())
tester = fault.Tester(PE_magma)
i0 = 0
i1 = 1

(1)
tester.circuit.inputs = (i0, i1)

(2)
tester.circuit.inputs = [i0, i1]

(3)
tester.circuit.inputs = input_t (*[i0, i1])

(4)
tester.circuit.inputs[0] = i0
tester.circuit.inputs[1] = i1

None of these options are currently working. It seems like these features just need to be implemented in fault, but which options seems like the best way to do this?

[leonardt]

Fault supports poking/expecting on tuples using either tuple syntax, e.g tester.circuit.I = (0, 1) or dictionary syntax, e.g. tester.circuit.I = {"x": 0, "y": 1} (this is actually old naming scheme, so this is how you would poke product types). Here's two examples: https://github.com/leonardt/fault/blob/master/tests/test_tester/test_core.py#L724-L767

Is the Peak compiled circuit encoding the tuple as a magma Bits? If so, then the above feature won't work since fault will think it's just a BitVector, so you would need to use some encoder to poke the tuple

[leonardt]

Confirmed, it looks like the magma circuit uses an assembled type, @cdonovick any thoughts on the best way to support this? The assembled type has access to the assembler that presumably could be used to encode the input value. The main concern is whether fault should build support in for this, or if Peak should define a sub-class of fault's tester that extends the value logic to support these assembled types. We may need to reorganize fault to make this kind of extension easier.

[rdaly525]

@leonardt, I think it depends how much you want fault to support MagmaAssembledADT. I personally think that being able to use arbitrary ADTs in magma/fault independent of peak would be very useful. But defining a peak-based fault tester also seems reasonable.

[leonardt]

Arbitrary ADT support would require a core ADT type in magma, right now MagmaAssembledADT doesn't use a core magma type by design (it uses the type protocol instead). Perhaps we can add a way to convert constant values to type protocol API, maybe this will work with "_get_magmavalue" (we can try constructing a value using the type of the port, then call _get_magmavalue to get a corresponding magma const)