m-labs / nmigen

A refreshed Python toolbox for building complex digital hardware. See https://gitlab.com/nmigen/nmigen
https://nmigen.org
Other
658 stars 57 forks source link

Array names revert to "$signal" #281

Closed mszep closed 4 years ago

mszep commented 4 years ago

[I'm a nmigen novice, so apologies if I'm doing something silly]

I'm trying to test a combinatorial module function that returns an array of Signals.

In my test functions, I have been iterating over sim._state.pending to find all signals by name and check if they have the right values, but this seems to fall down when the result is an Array.

Here is a minimal example:

from nmigen import Module, Signal, Elaboratable, Cat, Array
from nmigen import signed
from nmigen.back import pysim

class ArrayTest(Elaboratable):
    def __init__(self, num_bits):
        self.x = Signal(num_bits)
        self.y = Signal(num_bits)
        self.res = Array([Signal(signed(num_bits)) for _ in range(2)])

    def elaborate(self, platform: str):
        m = Module()
        m.d.comb += [self.res[0].eq(self.x+1)]
        m.d.comb += [self.res[1].eq(self.y+1)]
        return m

def test_arraytest():

    args = {'x': 12, 'y': 5}

    def python_function(x, y):
        return [x+1, y+1]

    res_python = python_function(**args)
    print("res_python: {}".format(res_python))

    block = ArrayTest(num_bits=64)

    sim = pysim.Simulator(block)

    def process():
        for key, val in args.items():
            yield block.__getattribute__(key).eq(val)

    sim.add_process(process)

    sim.run()

    for signalstate in sim._state.pending:
        print(signalstate.signal.name, signalstate.curr)

test_arraytest()

for me this prints

res_python: [13, 6]
$signal 6
$signal 13
y 5
x 12

However, I was expecting to see something like res [13, 6].

I guess what's happening is my loop is iterating over signals, whether or not they're part of an Array, and for those that are, there is no explicit name and therefore it defaults to $signal (as can be seen in ast.py).

It may well be the case that I'm testing my modules in a backward way (please tell me!) but I can't figure out a simple way to get at the result of res and check the values programmatically. Is this a bug? Is there a better way to do what I'm trying to do?

Thoughts appreciated.

whitequark commented 4 years ago

In my test functions, I have been iterating over sim._state.pending to find all signals by name

This is definitely not the right way to write testcases. The object sim._state is internal and I can and will break its interface without any consideration of downstream code.

You can get the value of any signal in res by writing a process that performs e.g. yield res[0]; the print statement you want can look something like print("res: [{}]".format(", ".join((yield block.res[n]) for n in len(block.res)).

mszep commented 4 years ago

Thanks for your quick response!

Unfortunately I can't figure out how to call it -- I presume you mean range(len(block.res)), but even with that fixed, I still get TypeError: sequence item 0: expected str instance, Signal found

whitequark commented 4 years ago

Oh wow that's a fascinating Python syntax edge case: turns out you can't really use yield in list comprehensions, and if you do, the results are quite bizarre:

>>> [(yield 1) for x in range(10)]
<generator object <listcomp> at 0x7ff1cb2815e8>
>>> list((yield 1) for x in range(10))
[1, None, 1, None, 1, None, 1, None, 1, None, 1, None, 1, None, 1, None, 1, None, 1, None]
>>> [1 for x in range(10)]
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
whitequark commented 4 years ago

Anyway, this works:

def test_arraytest():
    block = ArrayTest(num_bits=64)

    sim = pysim.Simulator(block)

    def process():
        yield block.x.eq(12)
        yield block.y.eq(5)
        yield pysim.Settle()
        print("res: [{}, {}]".format((yield block.res[0]), (yield block.res[1])))

    sim.add_process(process)

    sim.run()
mszep commented 4 years ago

Yup, works, thank you so much for your help!

jordens commented 4 years ago

You can certainly use yield in list comprehensions. The confusion you see is from two generators, the (yield 1) and the generator expression (_ for x in range(10)). Yield outside a function is depecated/a bug.

>>> def f(): yield from [(yield 1) for _ in range(10)]
... 
>>> list(f())
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1]

This is by the way another reason to use await/async as it gives you a cleaner and arguably more robust separation between async IO and language constructs like generators.