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.

Sign up for GitHub

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

Jump to bottom

Array names revert to "$signal" #281

Closed
mszep opened this issue Dec 11, 2019 · 6 comments
Closed

Array names revert to "$signal" #281

mszep opened this issue Dec 11, 2019 · 6 comments
Labels
question

Comments

@mszep
Copy link

mszep commented Dec 11, 2019 •
edited

[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
Copy link
Contributor

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

mszep commented Dec 11, 2019

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

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

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

mszep commented Dec 11, 2019

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

@mszep mszep closed this as completed Dec 11, 2019
@jordens
Copy link
Member

jordens commented Dec 11, 2019

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.

Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@whitequark @mszep @jordens