mciepluc / cocotb-coverage

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

randomize_with() fails #28

Closed sjalloq closed 4 years ago

sjalloq commented 4 years ago

Hi,

I've got the following Randomized class with two variables, dac_max and dac_min, that I want to constrain. The class definition looks like:

      self.add_rand("dac_max",         list(range(512)))
      self.add_rand("dac_min",         list(range(512)))
      self.add_constraint(lambda dac_max, dac_min : dac_max > dac_min   )

and I'm able to add further constraints without issue. Such as:

      self.add_constraint(lambda dac_max          : dac_max == 511      )

However, if I try to add a constraint with a call to randomize_with(), I get an exception raised. What is wrong with the following usage of randomize_with()?

   tx.randomize_with(lambda dac_max : dac_max == 384, lambda dac_min : dac_min == 32)

Shareef.

mciepluc commented 4 years ago

Hi @sjalloq

What is the exception you are getting?

sjalloq commented 4 years ago

Sorry, that would be useful wouldn't it...

# File "/home/shareef/projects/electrat-digital/work/cdr/tb/cdr_fsm/tests/test_cdr_fsm.py", line 419, in my_test
# tx.randomize_with(lambda dac_max : dac_max == 384, lambda dac_min : dac_min == 32)
#  File "/home/shareef/virtualenvs/electrat-digital/lib/python3.6/site-packages/cocotb_coverage/crv.py", line 321, in randomize_with
#    raise Exception("Could not resolve implicit constraints!")
#    Exception: Could not resolve implicit constraints!
mciepluc commented 4 years ago

Looks like bug... The usage is correct. The randomize_with call should override existing constraint on variable "dac_max". I will take a look next week. If you could prepare a failing testcase (under tests/test_crv/crv_test.py) that would be very helpful.

mciepluc commented 4 years ago

@sjalloq the fix has been commited. Actually the problem was totally different - related to incorrect usage of numpy.random.choice. When it was changed, number of other issues appeared, but hopefully all is stable now. Test suite was extremely helpful not to break already working stuff....

sjalloq commented 4 years ago

Hi,

thanks for this. Sorry, not looked at Cocotb for a while and only just started looing at your test cases. I think you added one for this test already.

In terms of updating my pip installation, it doesn't look like there's a new release. Should I just copy the latest GitHub crv.py over my local one?

mciepluc commented 4 years ago

@sjalloq new pip release will require a bit more time to prepare. If you want to use pip for installing version directly from github, try this: pip install git+https://github.com/mciepluc/cocotb-coverage@master (may require piror uninstalling already installed version)

cmarqu commented 4 years ago

The uninstalling can maybe be skipped with the option --upgrade.

sjalloq commented 4 years ago

@mciepluc , any plans to uprev the pip version soon? Just thinking whether I need to script the pip upgrade or not as I'd forgotten about the requirement and spent a while debugging the other day.

mciepluc commented 4 years ago

Hopefully the upgrade will come with 1.1 release estimated end of Feb for now.