-
Notifications
You must be signed in to change notification settings - Fork 56
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
CoveragePkg: RandCovPoint is randomly selecting bins with AtLeast=0 #26
Comments
I will look at it. It wasn't designed to have bins with a weight of 0. There are other contributing factors that may come into play. Are you finding any issues with non-zero numbers? |
I don't understand your question. The bins with positive |
|
Hi Ryan, I have reviewed the code and found two issues that impact this. First, every new bin is assumed to have a percent coverage of 0.0. Second the percent coverage calculation assumes that the AtLeast value is 1 or greater. It should be fairly straight forward to fix the code. I should have code for you to test shortly. |
What do you expect to happen when AtLeast is 0 and coverage goes beyond 100%? Should it start generating those values or should it never generate those values? How about if AtLeast is negative? Should a negative AtLeast value generate an Alert ERROR or FAILURE? |
I have changed how PercentCov is calculated. It now accounts for values of 0 or less than 0. If the value is 0, the PercentCov is set to 100.0. Hence, it should be randomly generated after all coverage goals have been met. If the value is less than 0, it sets the value to real'right. Hence, these values should never randomly be generated. I also made it easy to revise by having all of the calculations call the same function. This is a weakly tested development fix. It passes the current regression suite, however, the regression suite needs to be supplemented with a test that sets an AtLeast value of 0 to a bin with other bins set to non-zero. |
My understanding is that normal bins (AtLeast >= 1) will be generated by RandCovPoint until their coverage reaches 100%. I assume they can go over 100% and will still not be returned by RandCovPoint. I expect the AtLeast=0 bins to behave the same way. Having AtLeast < 0 doesn't make sense to me. Coverage talks about things that should happen. I guess you do have illegal bins and such to track things that shouldn't happen. But how is AtLeast=-1 different from AtLeast=-2? My first inclination is to put a VHDL assert of severity level FAILURE if AtLeast is less than 0. Your solution also makes sense. |
I agree, AtLeast <0 probably needs an assert. Also I am not happy with AtLeast = 0 being 100%. This has some negative side effects with thresholding. I think it needs to be real'right to prevent this. That would mean that AtLeast=0 means never randomize this item - even after all other items reach 100%. What does AtLeast of 0 mean to you? |
This may be a shortage in my understanding of how things should work. Consider the following partial example.
In a somewhat more complex example, I think I am seeing RandCovPoint generate a bin that has an AtLeast of 0 while other bins have AtLeast > 0 that haven't been covered yet. Is this expected?
The text was updated successfully, but these errors were encountered: