amiq-consulting / fc4sc

A header only C++11 library for functional coverage
Apache License 2.0
36 stars 15 forks source link

coverage_report/report.py considers OTHER bin type in result calculation #24

Open edcote opened 2 years ago

edcote commented 2 years ago

I expect 100% in covergroup and successfully achieve it using gui/index.html. When using coverage_report/report.py, I hit < 100%. The culprit is bins with type ignore. When I manually report this bin from xml report and re-run report.py, I hit 100%.

<ucis:coverpointBin name="OTHER" type="ignore" alias="0">
  <ucis:range from="32768" to="2147483647">
    <ucis:contents coverageCount="0" />
  </ucis:range>
</ucis:coverpointBin>
nsdjg commented 2 years ago

What do you mean, you "manually report this bin"? Are you changing the `type="ignore"' to 'type="report"'?

edcote commented 2 years ago

No. I remove the XML block altogether.

nsdjg commented 2 years ago

Literally, delete this entire entry?

<ucis:coverpointBin name="OTHER" type="ignore" alias="0">
  <ucis:range from="32768" to="2147483647">
    <ucis:contents coverageCount="0" />
  </ucis:range>
</ucis:coverpointBin>
nsdjg commented 2 years ago

FWIW, I'm working on this.

To simplify things, I took the output of the fir example and cut the second/empty covergroup instance stimulus_coverage_1. Ran the code like so and got the following result as a level set.

$ python3 report.py --xml_report JFD/fir.xml --yaml_out JFD/foo.yml 

  Overall Summary:

    Total Coverage:  81.25

  Module Summary:

      62.50 : output_coverage

      62.50 : output_coverage_1

          75.00 : data_ready_cvp 
          50.00 : output_valid_cvp 
     100.00 : shift_coverage

     100.00 : shift_coverage_1

         100.00 : shift_cvp 
      81.25 : stimulus_coverage

      81.25 : stimulus_coverage_1

          80.00 : values_cvp 
         100.00 : reset_cvp 
         100.00 : input_valid_cvp 
          45.00 : reset valid 

Then I modified the xml file further to change the values_cvp:max to be of type ignore rather than default. (FWIW, values_cvp consists of zero, lows[0-2] and max. Max is the one with zero coverage resulting in values_cvp = 80%) This results in all three types of coverpoint bins being in the file: illegal(1x), default(15x - not counting the cross entries for reset_valid) and ignore(1x).

The resulting diff of the xml file looks like this:

11:08 $ diff fir.xml.orig fir_with_ignore.xml | less
110c110
<           <ucis:coverpointBin name="max" type="default" alias="0">
---
>           <ucis:coverpointBin name="max" type="ignore" alias="0">
210,277d209
<           <ucis:userAttr key="20" type="int"/>
<         </ucis:cross>
<       </ucis:cgInstance>
.. stuff truncated since its just <ucis:cgInstance> to </ucis:cgInstance> removal.

Rerunning the code gives:

python3 report.py --xml_report JFD/fir_with_ignore.xml --yaml_out JFD/foo.yml 

  Overall Summary:

    Total Coverage:  81.25

  Module Summary:

      62.50 : output_coverage

      62.50 : output_coverage_1

          75.00 : data_ready_cvp 
          50.00 : output_valid_cvp 
     100.00 : shift_coverage

     100.00 : shift_coverage_1

         100.00 : shift_cvp 
      81.25 : stimulus_coverage

      81.25 : stimulus_coverage_1

          80.00 : values_cvp 
         100.00 : reset_cvp 
         100.00 : input_valid_cvp 
          45.00 : reset valid

Which is expected since max is of type ignore and the original code does not obey the directive.

I modified fc4sc/ucis_parser.py and report.py. Creating my own versions and reran the code on the modified xml file. The mods are not pretty but the end result is this:

  Overall Summary:

    Total Coverage:  83.85

  Module Summary:

      62.50 : output_coverage

      62.50 : output_coverage_1

          75.00 : data_ready_cvp 
          50.00 : output_valid_cvp 
     100.00 : shift_coverage

     100.00 : shift_coverage_1

         100.00 : shift_cvp 
      89.06 : stimulus_coverage

      89.06 : stimulus_coverage_1

         100.00 : values_cvp 
         100.00 : reset_cvp 
         100.00 : input_valid_cvp 
          56.25 : reset valid 

In this case, the value_cvp goes to 100% since the 0 rated coverage for values_cvp:max is ignored. The cross reset_valid which also uses values_cvp also increases.

If this is what is intended, I can upload the diff.