weighted distribution functionality doesn't seem to work properly

[Amirk97]

So I have set out a simple distribution function as below:

@vsc.constraint
def change_rand(self): 
    vsc.dist(self.enable_change, [
        vsc.weight(0, 95),
        vsc.weight(1, 5)])

but the randomized values don't seem to obey the weighted constraint that I was expecting, just by eyeballing the results things seem to be off. I have attached my log file for the mentioned variable.

f.txt

[alwilson]

That distribution does seem off, and close enough to a uniform distribution for a single bit that my gut says the constraint isn't even being used. Do you have a standalone test case you can share that repeats this? I suspect you've got an indentation issue where the constraint is getting defined within __init__ or outside the randobj class and it's not actually being parsed, but I ask that only b/c I ran into that exact same issue trying to reproduce this from your snippet! I got worried that my recent distribution fixes broke something...

If you call randomize(debug=True) you can see what constraints were parsed. It's not always the easiest to read, but at the beginning of the output you should be able to see pyvsc spit back out the fields and constraints it saw. My guess is that the enable_change field and that it's unconstrained.

Here's a working example of your dist:

import vsc
from collections import defaultdict

@vsc.randobj
class MyObj(object):

    def __init__(self):
        self.enable_change = vsc.rand_bit_t()

    @vsc.constraint
    def change_rand(self):
        vsc.dist(self.enable_change, [
            vsc.weight(0, 95),
            vsc.weight(1, 5)])

o = MyObj()
heatmap_o = {"enable_change": defaultdict(int)}

num_iters = 1000
for _ in range(num_iters):
    o.randomize(debug=False)
    heatmap_o["enable_change"][int(o.enable_change)] += 1

def print_report(title, class_name, heatmap_data):
    print("\n" + "="*60)
    print(f"      {title}")
    print("="*60)
    print(f"Class:          {class_name}")
    print(f"Total Runs:     {num_iters}")
    print("="*60 + "\n")

    for attr_name, value_counts in heatmap_data.items():
      print(f"--- Distribution for: {attr_name} ---")
      sorted_items = sorted(value_counts.items(), key=lambda item: (-item[1], str(item[0])))
      max_val_len = max(len(str(val)) for val in value_counts.keys()) if value_counts else 10

      for value, count in sorted_items:
          percentage = (count / num_iters) * 100
          bar = '#' * int(percentage / 2) # Scale bar for readability
          print(f"    - {str(value):<{max_val_len}} : {count:<5} ({percentage:6.2f}%) {bar}")
      print("-"*(26 + len(attr_name)) + "\n")

print_report("REPORT for MyObj", "enable_change", heatmap_o)

Output:

============================================================
      REPORT for MyObj
============================================================
Class:          enable_change
Total Runs:     1000
============================================================

--- Distribution for: enable_change ---
    - 0 : 951   ( 95.10%) ###############################################
    - 1 : 49    (  4.90%) ##
---------------------------------------

[Amirk97]

Thanks for the investigation! well I checked your suggestions but none of them were the case in my example unfortunately. I should add that I have another constraint like this:

@vsc.constraint
def coeff_rand(self):
    with vsc.if_then(self.enable_change == 0):
        self.coeff_index == self.prev_coeff_index
    with vsc.else_then:
        self.coeff_index in vsc.rangelist(vsc.rng(0,24))

I don't know if that can have an effect. Then also I turned on the debug option and the result was osmething like this which I'm not sure is it's correct or not:
constraint change_rand {
enable_change in [0, 1];
(95 == 0) -> {
!((enable_change == 0)) }
(5 == 0) -> {
!((enable_change == 1)) }
}
constraint coeff_rand {
if ((enable_change == 0)) {
(coeff_index == 0);
} else {
coeff_index in [0..24];
}
}

I do have a testcase that recreates the problem, this is the repo:
https://github.com/Amirk97/IIR_IP_SystemVerilog.git
Latest commit HEAD: 0214579
and my test is repeatable with this command:
make cocotb PY_TESTCASE=test_rand_coeff

[alwilson]

Which version of pyvsc are you using? I recently made some improvements to distributions that may apply here. Specifically when constraints exclude distribution values. The PRs only recently got merged and I don't think there's been a pypi release yet. Can you try top-of-tree and see if that resolves it?

This is getting me thinking that pyvsc should have a way to reproduce randomization objects / calls as standalone files so that these issues are easier to share and debug.

[alwilson]

Also, when I was looking at the prev_coeff_index in item_basic I think you want to use the non-random variable definition vsc.uint8_t(0). You might find issues with the post_randomize updates not showing up in the constraints.
https://github.com/Amirk97/IIR_IP_SystemVerilog/blob/021457973187e8e8b56988aab5e6debb7654eb3d/Python_tb/item_basic.py#L58-L60

[Amirk97]

So this was my package version:
Name: pyvsc
Version: 0.9.4.17961244602

I used the vsc._uint8_t(0) and it still didn't affect the issue.
I pulled the ToT and then suddenly things seem to be actually working! so good job with that, I advise to release the changes sooner into the the newest version since this is the main feature of the package!

Aside from that I think this part is pretty doable:
"This is getting me thinking that pyvsc should have a way to reproduce randomization objects / calls as standalone files so that these issues are easier to share and debug."
I think this is pretty doable since the post_randomized method is also implemented and now all it takes is to print the properties into a file. Also, actually it can be a good idea to do that in a testcase for the library and then integrate that into the regression suite so whenever something silently breaks down the randomization functionality, you would get informed of that (If it's not already implemented!)

[alwilson]

The bit about the vsc._uint8_t(0) was unrelated to the distribution issue, but that I don't think your post-randomize will work as intended on the next randomize call because it's not a proper pyvsc variable. I think the coeff_index == prev_coeff_index won't work as intended using a python int type. We should probably make that a warning or something.

Yeah, there's a pretty good test suite that vscode seems to pick up automatically! Has made it very useful to test new features. That said I didn't get to adding a test to detect this very distribution issue...

@mballance My distribution fixes help in this issue as well. Time for a new release?