mciepluc / cocotb-coverage

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

Resetting the coverage database for each @cocotb.test #65

Open rbarzic opened 2 years ago

rbarzic commented 2 years ago

Hi I'm using cocotb with several test cases per files - something like this:

@cocotb.test()
def test1(dut):
   .....

@cocotb.test()
def test2(dut):
   .....

At the end of each test I'm calling coverage_db.export_to_xml to a different file (one xml file per test) I noticed that the coverage results are "cumulative" when running all the tests from one make command. (coverage results for test2 include the coverage of test1 ) That makes sense as the coverage_db is a global singleton Is there a clean way to reset the coverage database at the beginning of each test ?

jonpovey commented 2 years ago

I also have this situation and would like this feature.

mciepluc commented 2 years ago

Hi @rbarzic and @jonpovey Seems you are using multiple cocotb tests in a single "run". For me it is not a typical use case. Do you use cocotb-test framework or Makefiles? cocotb-test allows you basically to run each test in a separate thread and this problem does not exist.

rbarzic commented 2 years ago

We are using the standard Makefile flow with "modules" and multiple tests per module. We could look into cocotb-test but we have already 10+ projects with more than 300 tests in total. That's a lot of work. It will be much easier for us to make to generate a special target to run all tests separately. How difficult would it be to have a function/method to clear the database on demand ?

On Sat, Jan 15, 2022, 15:46 Marek Cieplucha @.***> wrote:

Hi @rbarzic https://github.com/rbarzic and @jonpovey https://github.com/jonpovey Seems you are using multiple cocotb tests in a single "run". For me it is not a typical use case. Do you use cocotb-test framework or Makefiles? cocotb-test allows you basically to run each test in a separate thread and this problem does not exist.

— Reply to this email directly, view it on GitHub https://github.com/mciepluc/cocotb-coverage/issues/65#issuecomment-1013694113, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGGD4SECGAZETCGGBRAW73UWGCGDANCNFSM5HWMMNMQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

mciepluc commented 2 years ago

Well I guess adding a database reset function is not a big deal...

mciepluc commented 2 years ago

Since CoverageDB inherites the dict type, maybe a dict built-in operation could be used? https://www.askpython.com/python/dictionary/delete-a-dictionary-in-python

mciepluc commented 2 years ago

@rbarzic @jonpovey can you confirm whether the given suggestion works?

Scafir commented 1 year ago

Hi @mciepluc ,

TD;LR:

@cocotb.test()
async def myTest(dut)
    global coverage_db
    coverage_db.clear()
    coverage_db = CoverageDB()
    global my_coverage
    my_coverage = coverage_section([...])

    @my_coverage
    def sample_function(addr, rw):
        pass

    [...]

Clearing the dictionary is not enough, as some work is done on instantiation of the CoverageDB, as well as on the coverage_section. It is thus necessary to re-instantiate them again. It is also necessary to use non-global sampling function, as it is not possible to re-decorate a global function when not in global scope (to my knowledge).

I am only starting with cocotb-coverage, but it seems to me that the most flexible option would be to wrap the coverage functionality in a class and to instantiate a new object for each test. That would allow the user to have a different one for each test, or even to track the coverage of different components at the same time. Though it would be a lot of work to implement I guess.

mciepluc commented 1 year ago

@Scafir Thanks a lot for your input. It is however difficult for me to follow you proposal. It would be good if you or someone else propose a use case that is failing, then I can think about the solution. Eventually, we can make the coverageDB non-global, but it is not how coverage works in SV, so I do not want to diverge here.

Scafir commented 1 year ago

Hi,

I understand your refusal, as it may help people transitioning. It is also your project, and you are free to guide it as you see fit.

My suggestion comes from the fact that I am more used to python development than systemverilog. Using an instance rather than a global would help people like me in the following way: