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

traceSignals incorrectly writes real value signal to VCD #348

Open
zlobriy opened this issue Nov 23, 2020 · 11 comments
Open

traceSignals incorrectly writes real value signal to VCD #348

zlobriy opened this issue Nov 23, 2020 · 11 comments
Labels

Comments

@zlobriy
Copy link

zlobriy commented Nov 23, 2020

traceSignals function incorrectly writes real value signal to VCD. Verilog Standard states that a real value signal in VCD dump should start with "r" or "R" identifier. But instead, MyHDL creates signal starting with "s". So GTKwave can't make an analog wave. A function _printVcdStr causes this behavior. Just change 's' to 'r'.

@zlobriy zlobriy added the bug label Nov 23, 2020
@NicoPy
Copy link
Contributor

NicoPy commented Nov 23, 2020

Can you provide a simple excerpt of code showing the problem ?

@zlobriy
Copy link
Author

zlobriy commented Nov 23, 2020

Here is a simple model of ideal Digital to Analog Converter (DAC).

from myhdl import modbv, Signal, delay, always, Simulation, traceSignals, instance, now

import numpy as np


def clock_source(clock, frequency):
    period = 1 / frequency / 10 ** (-9)  # ns
    half_period = delay(round(period / 2))

    @always(half_period)
    def clksrc():
        clock.next = not clock

    return clksrc


def digital_sin_source(clock, sine, frequency, bits):
    out_range = 2 ** (bits - 1)

    @always(clock.posedge)
    def sinsrc():
        sine.next = int(out_range * np.sin(2 * np.pi * frequency * now() / 10 ** 9))

    return sinsrc


def ideal_dac(clock, digital, analog, bits, amin=-1, amax=1):
    step = (amax - amin) / 2 ** bits
    subs = -amin if amin < 0 else amin

    @always(clock.posedge)
    def dac():
        analog.next = step * digital

    return dac


def test_ideal_dac():
    bits = 8
    clock = Signal(bool(0))
    digital = Signal(modbv(0, min=-(2 ** (bits - 1)), max=2 ** (bits - 1)))
    analog = Signal(float(0))

    clksrc = clock_source(clock, 100 * 10 ** 6)
    sine = digital_sin_source(clock, digital, 10 * 10 ** 3, bits=bits)
    dac = ideal_dac(clock, digital, analog, bits=bits)

    @instance
    def test():
        yield delay(10)

    return clksrc, sine, dac, test


traceSignals.filename = 'test'
test_inst = traceSignals(test_ideal_dac)
Simulation(test_inst).run(100000)

This code generates test.vcd file. Then I open it in GTKwave (I have tried versions 3.3.100 on Windows and 3.3.103 on Linux) and add all signals. The "analog" signal is a real value signal, so I choose in GTKwave: Data Format -> Analog -> Interpolated. But nothing happens. When I locally modified _printVcdStr function and install MyHDL using pip from local files all is ok.

@NicoPy
Copy link
Contributor

NicoPy commented Nov 25, 2020

This is a little bit more complicated than just modifying _printVcdStr().
I'm working on a fix.

@zlobriy
Copy link
Author

zlobriy commented Nov 25, 2020

Thank you!
As a temporary workaround modifying _printVcdStr() works in my case.

@NicoPy
Copy link
Contributor

NicoPy commented Nov 26, 2020

@zlobriy Which version of MyHDL do you use ?

@zlobriy
Copy link
Author

zlobriy commented Nov 26, 2020

I use MyHDL 0.11.

@NicoPy
Copy link
Contributor

NicoPy commented Nov 26, 2020

I use MyHDL 0.11.

With MyHDL 0.11 you should use @block decorator.
Your example code should be written like this :

from myhdl import block, modbv, Signal, delay, always, Simulation, traceSignals, instance, now

import numpy as np

@block
def clock_source(clock, frequency):
    period = 1 / frequency / 10 ** (-9)  # ns
    half_period = delay(round(period / 2))

    @always(half_period)
    def clksrc():
        clock.next = not clock

    return clksrc

@block
def digital_sin_source(clock, sine, frequency, bits):
    out_range = 2 ** (bits - 1)

    @always(clock.posedge)
    def sinsrc():
        sine.next = int(out_range * np.sin(2 * np.pi * frequency * now() / 10 ** 9))

    return sinsrc

@block
def ideal_dac(clock, digital, analog, bits, amin=-1, amax=1):
    step = (amax - amin) / 2 ** bits
    subs = -amin if amin < 0 else amin

    @always(clock.posedge)
    def dac():
        analog.next = int(step * digital)

    return dac

@block
def test_ideal_dac():
    bits = 8
    clock = Signal(bool(0))
    digital = Signal(modbv(0, min=-(2 ** (bits - 1)), max=2 ** (bits - 1)))
    analog = Signal(float(0))

    clksrc = clock_source(clock, 100 * 10 ** 6)
    sine = digital_sin_source(clock, digital, 10 * 10 ** 3, bits=bits)
    dac = ideal_dac(clock, digital, analog, bits=bits)

    @instance
    def test():
        yield delay(10)

    return clksrc, sine, dac, test

tb = test_ideal_dac()
tb.config_sim(trace=True, timescale='1ns', filename="test", tracebackup=False)
tb.run_sim(100000)

@zlobriy
Copy link
Author

zlobriy commented Nov 26, 2020

Ok, I will use @block decorator. But how it helps with incorrect VCD? I had run an example from your last post, opened GTKwave, added waves, and see the same behavior as in the past. When I changed all lines in VCD which started with 's' to start with 'r' all going ok in GTKwave.

@NicoPy
Copy link
Contributor

NicoPy commented Nov 27, 2020

Sorry for not being clear enough on that point. The syntax you use is deprecated. You should use @block syntax to get MyHDL full functionalities (and future proof code). This is uncorrelated with the VCD problem you submitted.

Concerning the VCD problem, the quick fix you use (modifying _printVcdStr()) works in your case but be warned that this can break other cases.

@zlobriy
Copy link
Author

zlobriy commented Nov 27, 2020

Ok, I got it. I just started using MyHDL, probably examples from where I copied some code that was old. Now I use the only new syntax.

In my case, I have in VCD only bits and real values. I suppose that this quick fix can break the case which has string variables, an FSM state names, for example.

@NicoPy
Copy link
Contributor

NicoPy commented Dec 4, 2020

I suppose that this quick fix can break the case which has string variables, an FSM state names, for example.

Correct.

NicoPy added a commit to NicoPy/myhdl that referenced this issue Dec 4, 2020
hramon pushed a commit to hramon/myhdl that referenced this issue Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants