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

Distributions have higher priority than soft constraints #191

Closed
alwilson opened this issue Sep 4, 2023 · 1 comment
Closed

Distributions have higher priority than soft constraints #191

alwilson opened this issue Sep 4, 2023 · 1 comment

Comments

@alwilson
Copy link
Contributor

alwilson commented Sep 4, 2023

While looking back into constraint caching I noticed that distributions result in the final model having a very high priority soft constraint to ensure that the winning value take higher priority than any soft constraints. But the swizzler randomization also looks out for these distributions and will try to obey the weights. I wanted to see what purpose it had so I commented it out and found that the test_soft_dist_priority fails.

def test_soft_dist_priority(self):
"""Ensures that dist constraints take priority over soft constraints"""
@vsc.randobj
class my_item(object):
def __init__(self):
self.a = vsc.rand_bit_t(8)
self.b = vsc.rand_bit_t(8)
@vsc.constraint
def valid_ab_c(self):
self.a < self.b
vsc.soft(self.a > 5) #A
@vsc.constraint
def dist_a(self):
vsc.dist(self.a, [
vsc.weight(0, 10),
vsc.weight(1, 10),
vsc.weight(2, 10),
vsc.weight(4, 10),
vsc.weight(8, 10)])
hist = [0]*9
item = my_item()
for i in range(100):
item.randomize()
hist[item.a] += 1
self.assertGreater(hist[0], 0)
self.assertGreater(hist[1], 0)
self.assertGreater(hist[2], 0)
self.assertGreater(hist[4], 0)
self.assertGreater(hist[8], 0)

That got me thinking what it did in SV, so looking at the SystemVerilog LRM - 1800-2017 the Distribution section (18.5.4) seems to indicate that satisfying constraints, even soft constraints, comes before satisfying distribution weights. With the exception of the zero weight case. PyVSC adds a hard constraint for the zero weight case and an IN constraint for the possible values, which I think is correct, but I don't think it should cause non-zero weights to take priority over soft constraints.

If there are constraints on some expressions that cause the distribution weights on
these expressions to be not satisfiable, implementations are only required to satisfy the constraints. An
exception to this rule is a weight of zero, which is treated as a constraint.

I wrote up an SV testcase and tried it out, and it only randomizes a to 8, which seems to match up with the LRM:

class test_class;
    rand logic[7:0] a;
    rand logic[7:0] b;

    constraint addr_c {
        a < b;
        soft a > 5;
        a dist {
            0 :/ 10,
            1 :/ 10,
            2 :/ 10,
            4 :/ 10,
            8 :/ 10
        };
    };
endclass

module top;
    initial begin
        int mybins[];

        test_class test0;
        test0 = new();

        mybins = new[9];

        for (int j = 0; j < 100; j++) begin
            void'(test0.randomize());
            mybins[test0.a] += 1;
        end
        $display("%p", mybins);
    end
endmodule

Output:

'{0, 0, 0, 0, 0, 0, 0, 0, 100} 

Is this a specification bug? If so it seems like an easy fix and I think it would make constraint caching easier since the soft constraints wouldn't have to be regenerated and re-solved on each call, potentially.

mballance added a commit that referenced this issue Sep 26, 2023
- (#191) - Fix from @alwilson to ensure proper priority of dist vs soft constraints
- (exp) - Add experimental covergroup callback

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
@mballance
Copy link
Member

Fixed with PR #194

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants