cocotb-test sim_build directory location

[alexforencich]

I just ran in to an issue with cocotb-test that needs to be addressed. I have a directory structure that looks something like this

root
  +- rtl
  |   +- mod1.v
  |   +- mod2.v
  +- tb
      +- test_mod1
      |   +- Makefile
      |   +- test_mod1.py
      +- test_mod2
          +- Makefile
          +- test_mod2.py

If I run one of the makefiles or if I run py.test from within test_mod1 or test_mod2, everything works fine, and the sim_build_* directories are created inside the test_mod1/test_mod2 directories alongside the test scripts. However, if I run py.test from the tb directory that's up one level, all of the sim_build directories are created there and there are all sorts of conflicts.

The problem is the first line of Simulator.__init__:

self.sim_dir = os.path.join(os.getcwd(), sim_build)

There are two main issues here: first, if somebody provides an absolute path for sim_build, things will break, so detecting that and preserving an absolute path would be a good idea. Second, using os.getcwd like that is causing the sim_build directories from the tests in the subdirectories to all end up in the same location. I'm not sure what the best solution would be for that. One option is to do the same thing as the lib directory:

self.lib_dir = os.path.join(os.path.dirname(cocotb.__file__), "libs")

There may be other options as well, perhaps there is a way to set up pytest to change the cwd. However, I do not like passing any command line arguments to pytest - everything should run correctly if pytest is run with no arguments.

[themperek]

if I run py.test from the tb directory that's up one level, all of the sim_build directories are created there and there are all sorts of conflicts.

What exactly? I would think it should be ok if it is the same design all the time? This would be potentially also desirable to compile sources once.

first, if somebody provides an absolute path for sim_build, things will break, so detecting that and preserving an absolute path would be a good idea.

Yes. This is a bug.

Second, using os.getcwd like that is causing the sim_build directories from the tests in the subdirectories to all end up in the same location. I'm not sure what the best solution would be for that.

I am also not sure how to do this better. cocotb.__file__ can be in installed in the system and I do not think this is good idea to put compiled files there (often no permission also)

[alexforencich]

What exactly? I would think it should be ok if it is the same design all the time? This would be potentially also desirable to compile sources once.

The problem is that each directory tests a different module, and all of the sim_build directories are called sim_build_[datawidth] or something, so all of the testbenches for datawidth 8 for many completely different modules end up pointed at the same directory, and basically it's a crapshoot which one actually runs correctly (well, if any of them run correctly).

[themperek]

What exactly? I would think it should be ok if it is the same design all the time? This would be potentially also desirable to compile sources once.

The problem is that each directory tests a different module, and all of the sim_build directories are called sim_build_[datawidth] or something, so all of the testbenches for datawidth 8 for many completely different modules end up pointed at the same directory, and basically it's a crapshoot which one actually runs correctly (well, if any of them run correctly).

This looks like a bit special case but it still should work (for Icarus)? The design is compiled based on toplevel name so every module should make a new file?

cocotb-test/cocotb_test/simulator.py

Line 305 in 76dc1d6

self.sim_file = os.path.join(self.sim_dir, self.toplevel + ".vvp")

[alexforencich]

eh, maybe that would work, but that's pretty hackish and I'm planning on using other simulators as well, particularly verilator. I think it's a bug that the build files get dumped in whatever directory you happen to run py.test from. The issue is that py.test will recurse into subdirectories looking for tests, but then execute them with the working directory of wherever py.test was run from. In my case, I want to be able to run py.test within one subdirectory to run all of the tests for one module, or go up one level and run py.test to run the tests for all of the modules. My ideal solution would be to do the same thing as the libs directory and make it relative to __file__ of the script that called cocotb_test.simulator.run. So not cocotb.__file__, but perhaps extracting something from the stack. Or perhaps just fixing the absolute path problem so I can pass in an absolute path for sim_build that's derived from dirname(__file__).

[themperek]

Or perhaps just fixing the absolute path problem so I can pass in an absolute path for sim_build that's derived from dirname(file).

I can do this immediately. And you can do sim_build=dirname(__file__)

perhaps extracting something from the stack

I have tried something similar already.

cocotb-test/cocotb_test/run.py

Lines 24 to 28 in f0be378

	previous_frame = inspect.currentframe().f_back
	(run_module_filename, _, _, _, _) = inspect.getframeinfo(previous_frame)
	run_dir_name = os.path.dirname(run_module_filename)
	run_module_name = os.path.splitext(os.path.split(run_module_filename)[-1])[0]

I found it tricky because it is hard to cover all the uses cases (the user has to handle this now). We want also to run without a pytest or with customized simulations classes etc ... If you have some good idea let me know.

[gretzteam]

When using Pytest in parallel, I've been doing this to avoid colliding sim_build directory:

@pytest.mark.parametrize('xw', range(8,12))
@pytest.mark.parametrize('yw', range(20,22))
def test_widths(request, xw, yw):
    run(
      module='tb_cocotb',
      testcase='bringup',
      sim_build='sim_build_{}'.format(request.node.name).replace('[', '').replace(']', ''),
     ...

The string of .replace is quite hacky but without it I get directory names that were not so friendly for some tool I can't remember.
So far the generated names have been unique since they are driven from the parameters. Maybe this is useful?

[alexforencich]

well, that's a rather nice idea. Right now I am doing stuff like this for each module:

sim_build=f"sim_build_{axi_data_width}_{axis_data_width}"+("_unaligned" if unaligned else ""),

but if request.node.name includes the parameter values, then it would be a more general solution.

The problem I am running in to, though, is that the build directories from several different python test scripts are getting dumped into the same directory. I'm not sure if request.node.name would be unique across different scripts or not. At any rate, what I need to do is ensure sim_build ends up in the same directory as the python script, no matter what the working directory happens to be.

[gretzteam]

I'm not too familiar with pytest but there might be a way to get the calling script name or directory directly?

The issue I was having with the method above is each simulation would have to recompile the verilog. Many time the 'parameters' are only affecting the tb and do not mandate a recompilation. I was wondering if there shouldn't be a split between the directory holding the compiled output, and the directory where we dump simulation outputs.

[alexforencich]

In my case the parameters are all module parameters, as such the module needs to be recompiled for each set of parameters.

[alexforencich]

So #115 means absolute paths work for sim_build at least as a stop-gap, now the question is what to do with relative paths. Is using the working directory the best method? Is there a clean way to get pytest to change the directory when recursively running tests? Are there other options worth considering?

Right now I am experimenting with assigning sim_build like so:

sim_build = os.path.join(os.path.dirname(__file__),
    "sim_build_"+request.node.name.replace('[', '').replace(']', ''))

Seems to be working so far.

[themperek]

@alexforencich Not sure if can make everyone happy ;-(

My use-case is normally to have one design and multiple tests in multiple directories and compile once.
What should be the default 🤷‍♂️

One can make their own run() function wrapper and set their own defaults or derive simulator class.
Any good ideas welcome.

[alexforencich]

Yeah, true. My current use-case is testing libraries that contain a bunch of modules. One folder and one testbench python file per module, using cocotb-test to step through a decent set of parameters. Currently, I am running in to some pytest test collection issues due to some duplicate python file names, however - some of the myhdl testbenches have the same names as the cocotb testbenches. pytest recommends adding __init__.py files to the subdirectories in this case and doing so causes pytest to collect all of the tests, but then none of the cocotb tests run as cocotb fails to import the testbench modules. Rather annoying.

[themperek]

pytest to collect all of the tests, but then none of the cocotb tests run as cocotb fails to import the testbench modules. Rather annoying.

Add module path to python_search ?

cocotb-test/cocotb_test/simulator.py

Line 38 in 00d2b9a

python_search=None,

[alexforencich]

interesting idea, just tried setting python_search=os.path.dirname(__file__), but no dice, looks like all the cocotb tests failed again with a ModuleNotFoundError for the testbench module. I also tried changing the working directory before calling simulator.run with os.chdir, which also didn't work. However, it looks like there is also a work_dir parameter to simulator.run, and setting that to os.path.dirname(__file__) seems to have worked. Nice!

[themperek]

Heh. So I think work_dir is a legacy and should be removed. Do not see much use for this?

It should work with python_search. Maybe problem is that os.path.dirname(__file__) will return relative path?

Anyhow, I should document run() ..

[alexforencich]

I uploaded some of the tests I've been working on here, why don't you experiment with it and see if you can figure out what the correct solution is: https://github.com/alexforencich/cocotbext-axi/tree/master/tests

I tried setting both python_search and work_dir, setting python_search did nothing while setting work_dir seemed to enable cocotb to find the module in question. Not sure why work_dir would be legacy, it looks like that's used to set the working directory of the simulator process. If that's legacy, then how do you set the working directory for the simulator process?

[themperek]

So if I do python_search=[tests_dir], it seems to work for me.

The only drawback using work_dir is that some simulators make a lot of temporary files there.

If you make sim_build like bellow you dan use cocotb-clean -r to clean all sim_build folders:

    sim_build = os.path.join(tests_dir,
        "sim_build", request.node.name.replace('[', '-').replace(']', ''))

BTW: pytest-xdist seems to work so pytest -n no_of_processes

If that's legacy, then how do you set the working directory for the simulator process?

You are right. Was thinking of default to sim_build but it may be useful in some cases.

Would you like to set up some CI?

[alexforencich]

Oh, it needs to be a list. That makes sense, I'll swap that out then and leave work_dir alone. And yes, I am definitely using xdist, it makes running hundreds of tests bearable on my EPYC 7742 CPU.

I will definitely set up CI on that repo at some point, I have travis-ci set up on some of my other repos. If you have any input on that, I would be happy to take any suggestions. At least the last time I checked, travis-ci has an execution time limit of like an hour or something, but some of my tests run for 2 hours or more (lots of nested loops).

Also, I think work_dir may default to sim_build if it's not otherwise specified.

[themperek]

Do you think we can close this?
Any ideas?

[alexforencich]

I mean, with the sim_build absolute path bug fixed, setting that (and python_search) manually from __file__ works. But I think it would be useful to investigate doing that automatically via introspection of some sort to possibly reduce duplicate boilerplate testbench code.

[gretzteam]

As I alluded to in a comment above I think it would be good to have an option to differentiate between the directory where stuff is compiled to, and the directory where simulation output are stored. They could be made the same by default.

There are two types of pytest parametrization possible. One that affects the dut (like changing top level dut parameters, requiring a recompile) and one where tb/dit modes are parameterized. The latter doesn't need recompilation while outputs from each sim should be in a separate directory.
I can open a new PR is this makes sense.

[alexforencich]

Well, and there are also multiple simulators, maybe it would make sense to include the simulator in the build directory name so that running the same tests with multiple simulators don't result in conflicts, especially in a CI context.