mciepluc / cocotb-coverage

Functional Coverage and Constrained Randomization Extensions for Cocotb
BSD 2-Clause "Simplified" License
100 stars 15 forks source link

How to write distributions #34

Closed sjalloq closed 4 years ago

sjalloq commented 4 years ago

Hi,

I was just trying to write a test case for a distribution that doesn't seem to work and found a bunch of examples in your tests that I have no clue about.

What does the following constraint do in your test_distributions_1?

d2 = lambda y: 2 * y

Is there some implied missing syntax here because it doesn't seem to make sense.

Then, for a standard distribution, I was trying to force a 50/50 distribution of reads/writes in my transaction but it seems to tend towards 33:66. Is the following not correct?

dist = lambda rnw: 0.5 if rnw else 0.5 where rnw is my readNotWrite flag.

sjalloq commented 4 years ago

Here's an example:

from cocotb_coverage.crv import Randomized

class RandTrxn(Randomized):
   """
   """
   def __init__(self):
      Randomized.__init__(self)
      self.rnw  = 0
      self.addr = 0

      self.add_rand("rnw", range(2))
      self.add_rand("addr", range(32))

      # Constrain the channel distribution
      self.add_constraint(lambda rnw: 0.5 if rnw else 0.5)
      self.add_constraint(lambda addr,rnw: addr < 31 if rnw else addr < 16)

   def __repr__(self):
      return "rnw=%s addr=%s"%(
         self.rnw, self.addr)      

reads = 0.0
writes = 0.0
spi = RandTrxn()
for _ in range(1000):
   spi.randomize()
   if spi.rnw == 1:
      reads +=1
   else:
      writes +=1

print("read precentage = %s"%(reads/(reads+writes)*100.0))
mciepluc commented 4 years ago

@sjalloq It is good to finally have this documented :) The distributions work this way, that you assign a weight to the particular solution. So with the example: d2 = lambda y: 2 * y means that when y = 0, then assigned weight is 0, so this solution will never bee taken. And then, with each rising value of y the weight is higher, so high values will be preferred over low ones.

For your example: dist = lambda rnw: 0.5 if rnw else 0.5 You should simply get 50/50 distribution. If you get different results, please prepare a testcase and I will take a look.

sjalloq commented 4 years ago

@mciepluc The example I posted above shows the issue. Am I using it in an incorrect way or would you expect that to work? I've run the loop for ever increasing iterations and it never gets below 60%.

mciepluc commented 4 years ago

ok , I will test it

mciepluc commented 4 years ago

@sjalloq

The issue here is that you basically introduced two distributions. So you constrained rnw variable using two different functions. The solver tries to satisfy both, that's why what you get is not 50/50 on rnw variable. What you probably mean is to make sure rnw is randomized before second constraint is taken intro consideration. Using solve_order will do the job - it will force to first satisfy the rnw variable constraints only at the beginning.

            self.solve_order("rnw", "addr")

Please reply if this explanation is clear. I added testcase for this as well.

cmarqu commented 4 years ago

This seems to be a good addition to the docs.

sjalloq commented 4 years ago

@mciepluc thanks for this. So you're saying that the second constraint on the address also somehow constrains rnw? What is the constraint there?

Is this the same behaviour as UVM? It's been a while since I wrote any constraints but this seems strange.

mciepluc commented 4 years ago

@sjalloq, it is a different constraint/distributions mechanisms that SV. Basically, if you define two functions, two are treated as equivalent, the solver does not know which one has higher priority. So if you define one function which results in 50/50 distribution, and second with 10/90, the result will be 30/70 distribution. This is what is happening here. Your hard constraint lambda addr,rnw: addr < 31 if rnw else addr < 16 already reduces the search space and additionally you define 50/50 distribution on this search space. As a superposition, you are not getting 50/50 distribution. Again, please use solve_order function to make sure the functions are executed in the requested order.

sjalloq commented 4 years ago

I'm really struggling here with the correct syntax. :-( What's wrong with the following testcase.

What I'm trying to do is setup a default for a variable that I can override when needed. This is for a corner case where I just need to force a value. So I'd like to be able to call randomize() in all my tests aside from one where I'm specifically calling randomize_with(lambda override_en: override_en == 1)

def test_solve_order2():

    class RandTrxn(crv.Randomized):
        def __init__(self):
            crv.Randomized.__init__(self)
            self.dac_override = 0
            self.dac_override_en = 0

            self.add_rand("dac_override", list(range(512)))
            self.add_rand("dac_override_en", list(range(2)))

            self.add_constraint(lambda dac_override_en: dac_override_en == 0)
            def c_override(dac_override,dac_override_en):
                if dac_override_en == 1:
                    dac_override > 0
                else:
                    dac_override == 0
            self.add_constraint(c_override)

            self.solve_order("dac_override_en", "c_override")

    test = RandTrxn()
    test.randomize()

    assert test.dac_override == 0
mciepluc commented 4 years ago

@sjalloq That looks OK. if you call randomize_with(lambda dac_override_en: dac_override_en == 1) you should get dac_override = 0. If you call a simple randomize() you should get dac override in range 0 to 511. Are you getting a different result?

sjalloq commented 4 years ago

Sorry, should have said, it barfs with a ValueError.

https://github.com/sjalloq/cocotb-coverage/commit/ad358a8baf2ffc493ceed62e2436644a52954977

Hmm, that diff isn't that useful as I had to run dos2unix beforehand.

mciepluc commented 4 years ago

Another comment is that I wouldn't use a randomized variable in that case, I would use dac_override_en as a class member:


def test_solve_order2():

    class RandTrxn(crv.Randomized):
        def __init__(self):
            crv.Randomized.__init__(self, dac_override_en = 0)
            self.dac_override = 0
            self.dac_override_en = dac_override_en

            self.add_rand("dac_override", list(range(512)))

            def c_override(dac_override):
                if self.dac_override_en == 1:
                    dac_override > 0
                else:
                    dac_override == 0
            self.add_constraint(c_override)

    test = RandTrxn()
    test.randomize() #random result

    test2 = RandTrxn(dac_override_en = 1)
    test2.randomize() #dac_override should be 0
sjalloq commented 4 years ago

OK, thanks. I'll try that.

mciepluc commented 4 years ago

Sorry, I missed one thing - the function solve_order takes as arguments variables, not constraint names. Instead of self.solve_order("dac_override_en", "c_override"), you should have self.solve_order("dac_override_en", "dac_override"). I wonder if this function is needed here anyway... i think it is not. Can you try this?

sjalloq commented 4 years ago

That's how I tried it the first time but it also failed with an error so I assumed I had to supply the constraint name.

I think I like your suggestion of a class member better anyway.

sjalloq commented 4 years ago

There still seems to be something wrong with the syntax here:

def test_solve_order2():

    class RandTrxn(crv.Randomized):
        def __init__(self, dac_override_en=0):
            crv.Randomized.__init__(self)

            self.dac_override = 0
            self.dac_override_en = dac_override_en

            self.add_rand("dac_override", list(range(512)))

            def c_override(dac_override):
                if self.dac_override_en == 1:
                    dac_override > 0
                else:
                    dac_override == 0

            self.add_constraint(c_override)

    test = RandTrxn()
    test.randomize()

    assert test.dac_override == 0

gives

>   non_zero_weights = [x for x in weights if x > 0]
E   TypeError: '>' not supported between instances of 'NoneType' and 'int'
mciepluc commented 4 years ago

I think your constraint function does not return anything. I think it should be written this way:

            def c_override(dac_override):
                if self.dac_override_en == 1:
                    return dac_override > 0
                else:
                    return  dac_override == 0
sjalloq commented 4 years ago

schoolboyerror