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

Support for open source EDA tools from containers #221

Closed
wants to merge 1 commit into from
Closed

Support for open source EDA tools from containers #221

wants to merge 1 commit into from

Conversation

carlosedp
Copy link
Contributor

This PR adds suppor for running open source EDA tools directly from
containers. By default, tools are run from local host but added a
configurable parameter use_containers that enable running from
containers.

There is also a parameter container_daemon allowing the user to set
another daemon such as Podman.

The configuration comes from a centralized makefile that is imported
from other backends overriding the current defined tools by container
counterparts.

Adjusted the backends to use this makefile and call them via variables.

@mithro
Copy link

mithro commented Feb 24, 2021

FYI -- @acomodi - @kgugala

This PR adds suppor for running open source EDA tools directly from
containers. By default, tools are run from local host but added a
configurable parameter `use_containers` that enable running from
containers.

There is also a parameter `container_daemon` allowing the user to set
another daemon such as Podman.

The configuration comes from a centralized makefile that is imported
from other backends overriding the current defined tools by container
counterparts.

Adjusted the backends to use this makefile and call them via variables.
@olofk
Copy link
Owner

olofk commented Feb 28, 2021

Thanks for this. I know too little about containers myself to feel comfortable in reviewing this alone, but I have some questions

  1. Can you run Edalize itself + the tools in a container instead? I guess the tools would be available in the container then, so that you don't have to call out to docker for each tool. Of course, this would not work if you want to run the configure step on one machine and the build step on another. I'm not sure how those workflows usually work
  2. Do we only need to prefix the commands? If so, I wonder if we instead should just add the possibility to prefix commands instead of adding the docker prefix to the code.
  3. Do we need to hard code names of the containers? What if users want to use a different container? Also a bit scary to rely on something that can be removed at any point in time

@olofk olofk mentioned this pull request Feb 28, 2021
@olofk
Copy link
Owner

olofk commented Mar 12, 2021

Let's close this and focus on adding support for supplying a run command instead

@olofk olofk closed this Mar 12, 2021
@umarcor
Copy link
Contributor

umarcor commented Mar 15, 2021

  1. Can you run Edalize itself + the tools in a container instead?

This is possible, but non-idiomatic in the context of containers. Having all the tools available in a single environment is the approach of a Virtual Machine. Containers are more lightweight and tool specific. See https://hdl.github.io/containers/#_usage and librecores/docker-images#33.

I guess the tools would be available in the container then, so that you don't have to call out to docker for each tool.

The overhead of starting multiple containers is negiglible compared to the download time of a single huge image vs small tool specific images. All the images in hdl/containers share the same base. Therefore, when retrieving multiple images, you only need to get the unique layers, not the full size of the image.

Nevertheless, we also provide images which include multiple tools. edalize should work there already, because in that use case edalize is unaware of containers.

  1. Do we only need to prefix the commands? If so, I wonder if we instead should just add the possibility to prefix commands instead of adding the docker prefix to the code.

Supporting a prefix to the commands is equivalent to supporting the commands to have non-standard names or to be located in absolute locations. "Run this inside this container" is in some sense equivalent to "use this specific binary". The added complexity in the case of containers is binding the locations.

In other words, since Yosys supports custom command prefixes, users will need a prefix for using antmicro-yosys, quicklogic-yosys or mycustom-yosys, isn't it?

  1. Do we need to hard code names of the containers? What if users want to use a different container? Also a bit scary to rely on something that can be removed at any point in time

I think the container names should have default values but be customizable. I believe that @carlosedp's implementation is based on similar makefile based solutions:

For reference, PyFPGA supports specifying images and/or commands through a YAML file: https://github.com/PyFPGA/pyfpga/blob/main/examples/configs.yml. /cc @rodrigomelo9

Let's close this and focus on adding support for supplying a run command instead

If you mean #218, I feel it is not a replacement for this feature. Managing the directories and sharing them with the container is not something to be done manually... I agree that this PR could be reworked to make profit from a prefix feature. But I would recommend not to discard it.

@olofk olofk reopened this Mar 15, 2021
@olofk
Copy link
Owner

olofk commented Mar 15, 2021

Hi @umarcor. Thanks for the additional info. I don't understand though why #218 isn't enough to solve this? The idea would be that instead of edalize calling a tool directly, it would send the command-line to a script that creates the full command-line. A simple script could look like this

#!/usr/bin/python
import subprocess
import sys

print(sys.argv)
if sys.argv[1] == 'yosys':
    subprocess.call(['docker', 'run', 'hdlc/yosys']+ sys.argv[1:])
else:
    subprocess.call(sys.argv[1:])

I think it would make total sense to make a script that reads @rodrigomelo9's configs.yml file so we can just reuse the container and option names from there.

And to support the cases in #218, a script could e.g. call

if sys.argv[1] == 'xrun':
    subprocess.call(['nc', 'run', '-C', 'ncsim', '-Il',]+sys.argv[1:])

In my view, that would be the most flexible option. Or do I miss anything?

@umarcor
Copy link
Contributor

umarcor commented Mar 15, 2021

@olofk, your view is correct, and that's why I suggested that this PR can be reworked after #218 is merged. However, you are missing the fact that containers are primarily meant for isolation (not sandboxing tho). Therefore, by default no folder from the host is available inside the containers. That is, ['docker', 'run', 'hdlc/yosys'] will fail because it will start the container successfully but none of the files will be found.

Hence, the directory or list of directories need to be explicitly bind: ['docker', 'run', '-v', 'HOST_PATH:CONTAINER_PATH', 'hdlc/yosys'], where HOST_PATH is the absolute path of a directory on the host, and CONTAINER_PATH is where to mount it inside the container. -v can be repeated as many times as wanted. This is, indeed, the complex part of dealing with containers. If users of edalize have all the resources below a root, then it's easy. However, if dependencies/libs are spread all over the host, guessing which directories need to be mounted and probably rewriting the paths (if not relative) can be painful.

@umarcor
Copy link
Contributor

umarcor commented Mar 15, 2021

I think it would make total sense to make a script that reads @rodrigomelo9's configs.yml file so we can just reuse the container and option names from there.

Absolutely. I would love to see PyFPGA and edalize slowly integrating into each other. PyFPGA has already dealt with some of this container and directory issues, but the implementation is not complete yet. Some workflows are supported but others are untested. I believe it makes sense to make the container configuration part reusable by different project/task management tools.

@rodrigomelo9
Copy link
Contributor

Hi. I am totally open to discuss and redefine config.yaml if needed. I am creating a separated tool to solve the FOSS flow of PyFPGA (https://github.com/PyFPGA/openflow, totally WIP) where tools like Yosys, GHDL, ghdl-yosys-plugin, nextpnr (ice40 and ecp5), icestorm and prjtrellis are combined. The containers configurations will be performed there, and it will be heritaged by PyFPGA. So, is a great moment to define how the containers and the tools names will be specified :-D

@olofk
Copy link
Owner

olofk commented Mar 16, 2021

Great! Seems like we're all on the same page here. I get the complexity of sharing files between the source and hosts, but this seems to be something we can't easily solve in a central way and it has to be left to the users. For FuseSoC users (where Edalize is used as a backend) all files will be available in a clean build directory, so in that case it would be easy to share with the containers. For people using Edalize directly we have no clue where they keep their files so I guess the users must be able to specify that somewhere.

But from the perspective of Edalize I think it's pretty simple by now if we just support an external runner and handle all the complexity inside of that. And this seems like a good thing to share between pyfpga and Edalize (and HDLMake @garcialasheras?). I saw @rodrigomelo9's proposal for a simplified config structure and that looks fine to me.

So yes, let's rework this PR but instead of calling a container daemon, it will call a user-defined run command. In addition to that I would like to see something like https://github.com/PyFPGA/pyfpga/blob/main/fpga/tool/openflow.py#L56 split out to a separate script that constructs a command-line from a combination of configs.yml and a user-supplied command-line

@rodrigomelo9
Copy link
Contributor

In addition to that I would like to see something like https://github.com/PyFPGA/pyfpga/blob/main/fpga/tool/openflow.py#L56 split out to a separate script that constructs a command-line from a combination of configs.yml and a user-supplied command-line

I will work on that. I am thinking in a class (openflow.config) where you can specify a YAML file (PyFPGA/openflow#6), and employ the get_command(tool) method, which will return the complete command. Additionally, an env var could be read to specify do not to use an OCI container (as shortcut).

@olofk
Copy link
Owner

olofk commented Mar 28, 2021

Ok, so with f8b3f66 I would argue we have what we need for a container workflow. Thanks for kicking this off, but I think we can close this one now

@olofk olofk closed this Mar 28, 2021
@carlosedp
Copy link
Contributor Author

Sure, that was a nice solution.
The point now is how/where to maintain a good runme.py script :)

@umarcor
Copy link
Contributor

umarcor commented Mar 28, 2021

@olofk
Copy link
Owner

olofk commented Mar 29, 2021

@carlosedp I think we can ship an edalize_docker_launcher executable or something like that with edalize. Will also add support eventually to specify the launcher as a command-line argument. For FuseSoC users, it would then be something like fusesoc run <options> <core> --launcher=edalize_docker_launcher

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

Successfully merging this pull request may close these issues.

5 participants