byuccl / spydrnet

A flexible framework for analyzing and transforming FPGA netlists. Official repository.
https://byuccl.github.io/spydrnet
BSD 3-Clause "New" or "Revised" License
89 stars 21 forks source link

Not all example netlists working #153

Closed benglines closed 1 year ago

benglines commented 3 years ago

It seems that the example netlists are in the SpyDrNet repo, but when downloading from PyPI, not all are available for parsing them into SpyDrNet.

jacobdbrown4 commented 3 years ago

I took a look at the support files (the tar.gz and .whl files) for spydrnet on pypi.org and saw that not all of the example netlists were there. My guess is that we made a mistake when doing the latest release and those example netlists were somehow left out. When we do the next release, we should make sure those example netlists are included with the rest.

See https://pypi.org/project/spydrnet/#files

jacobdbrown4 commented 2 years ago

I went in and tried building the .whl and .tar.gz files for spydrnet again and then installing them to see if the missing netlists were there, but they weren't. I think I found the problem though.

When we generate the files, we run the command "python3 setup.py sdist bdist_wheel" and I guess the setup.py tells it what to do. In the setup.py file, there is a line that specifies the size of the example netlist file that can be included:

if os.path.isfile(filename) and os.path.getsize(filename) < 1024 * 10:

in the following code block:

folder_path = os.path.normpath(os.path.join(os.path.dirname(__file__), "spydrnet", "support_files"))
for filename in glob.glob(os.path.join(folder_path, "**", "*"), recursive=True):
    if os.path.isfile(filename) and os.path.getsize(filename) < 1024 * 10:
        example_edif_files.append("support_files/" + str(filename)[len(folder_path) + 1:].replace('\\', '/'))

I'm not sure why this constraint is there, but if we remove it or even make it bigger it will probably fix our problem. Maybe the additional netlists make the package too big?

jacobdbrown4 commented 2 years ago

We should probably change this line of code. Here are some suggestions from @wirthlin :

andrewmkeller commented 2 years ago

The line limiting the size of netlists in the package file was placed there intentionally. It is meant to prevent the built package from bloating in size. If not checked, we could accidentally make our package too large to place on pypi.

Check this out from pypi.org

https://pypi.org/help/#file-size-limit

Here is a helpful blog post about it as well.

https://www.dampfkraft.com/code/distributing-large-files-with-pypi.html

Here is a report on our utilization now:

image

We can definitely make the file bigger if we really want to, but I argue against it. I don't think it is necessary to make every all users download a large collection of netlists, most of which they will likely never use. I think it would unnecessarily weigh us down.

The original intent of including netlists in the first place was to give users a quick start and to provide lots of simple test cases that can be used to used to run a rapid set of tests.

Currently if users want access to the other netlists that are included in the repository, they can simply clone the repository and install spydrnet out of the repostory with pip install -e . from the spydrnet directory.

Another alternative would be to add a zip of example files to the release page on github or even just a seperate pypi package within our pypi project. I would like for the primary release of spydrnet to be a quick download, quick start.

The example netlists in are not likely to change drastically from release to release. Here is a fun thought, what if we did have a fun package like spydrnet_netlists that was just a collection of netlists with a handle for accessing them and then we added the package as an extra in the setup.py file? We can then maintain the collection of netlists seperate from core, but still keep them together.

https://stackoverflow.com/questions/56998851/optional-dependencies-for-pypi-packages

These are some ideas. I'll dig in and see what is best practice.

jacobdbrown4 commented 1 year ago

This will no longer be an issue once the example netlists are moved out of the python package. See #200