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

Merging board definitions into amaranth-boards #237

Open
tilk opened this issue Dec 6, 2023 · 5 comments
Open

Merging board definitions into amaranth-boards #237

tilk opened this issue Dec 6, 2023 · 5 comments
Labels

Comments

@tilk
Copy link

tilk commented Dec 6, 2023

I'm using Amaranth for university classes, where the students have access to DE1-SoC and DE2-115 boards. The board definition for DE1-SoC in amaranth-boards is very incomplete, and there is no definition for DE2-115. However, there are pull requests with pretty good definitions, but they are waiting there for over two years. I told the students to use the definitions from the pull requests, but this doesn't leave a good impression. Are there plans for maintaining this package and accepting community contributions?

@whitequark
Copy link
Member

The package is maintained: it is updated to track changes in Amaranth itself, issues (like this one) are monitored, and so on. The last commit was less than 2 months ago. The fact that the review queue is so long is separate from the question of whether it is maintained.

There are two related reasons that there are pull requests sitting in the queue for a long time:

  1. It takes an extreme amount of effort to review a board definition file, which is at the moment unjustifiable to take away from core development.
  2. Many of the submitted board definitions files are first contributions and do not pass our bar for quality.

I have always viewed it preferable to not have a contribution upstream at all rather than to bless a contribution that is misleading to those who will trust it. This applies in particular to board definitions, where an incorrect definition can very well lead to hardware damage. The path forward here would be twofold:

  1. Define a formal standard for what constitutes a "good enough" definition for amaranth-boards. This includes conventions of coding style, resource naming and structure, clear indication of completeness, as well as a formal sign-off process that ensures it is unlikely that we merge an erroneous board definition file that can damage someone's hardware.
  2. Recruit reviewers who will be ensuring the added definitions satisfy the standard above.

You are benefiting from Amaranth as an educator, and complain about a part of it not leaving a "good impression", but are you prepared to contribute back? We could resolve (1), but that will still leave us with a shortage of people for doing (2), and I do not expect to have any more time for doing board definition review than I do right now.

As a final note, I want to say that reviewing board definitions files is probably some of the most time-consuming, tedious, and thankless work of all I do and have ever done for Amaranth, and the only thing I really get back is complaints, so I'm not particularly motivated to do more of it in response to this issue.

@whitequark whitequark pinned this issue Dec 6, 2023
@tilk
Copy link
Author

tilk commented Dec 6, 2023

I'm ignoring the snarky and unnecessary remark and will focus on the heart of the matter only.

Yes, I'm aware of the issues involved in ensuring quality of board definitions. Unfortunately, amaranth-boards as it is now doesn't really serve its purpose, and the people who lose the most are the beginners, who don't really understand everything and want their boards to just work.

Part of the quality assurance could be solved by using automatic checks in Github workflows. Coding style can be checked by flake8, black, or other tools; I use them myself in my projects and find them to be of great help. Naming and structure could also be (at least partially) verified by a custom checker.

Checking correctness and completion is a harder problem. It would be best if the reviewer had physical access to the board in question, which is hard because some boards cost quite a lot. Checking with the board schematics and documentation is in theory sufficient, but in my experience hardware tends to do unexpected things sometimes. I would imagine that unpaid contributors wouldn't be really interested in doing all of that. So I think that the only way to have reasonable coverage of boards would be to leverage the community: the people interested in having the boards they own supported.

I would imagine that, other than the verified and checked board definitions, there could be a staging area of definitions which passed the immediate checks, but are not sufficiently verified for correctness and completeness. The community could then use these definitions, test them, and point out the issues. Moving a board definition from the staging area to the verified set could be done by accepting a review from a contributor with confirmed credentials.

I, personally, can offer my time to verify the definitions for the boards I mentioned, DE1-SoC and DE2-115, which I use actively for my classes.

@whitequark
Copy link
Member

I'm ignoring the snarky and unnecessary remark

Open source projects are made or broken by the relationships between the people building them. I have found the approach you have taken equally snarky and unnecessary, and I do not find opening with this response constructive or conducive to a good working relationship.

My experience of open source maintenance is that of interacting with expectation and entitlement. If you cannot recognize that then I don't think I will have a good experience working with you.

@tilk
Copy link
Author

tilk commented Dec 6, 2023

I'm deeply sorry that you feel offended by what I've written. I'm trying to communicate about the project not because of entitlement (how could I be, if I didn't pay a penny?), but because I care about your project and believe we could mutually benefit through collaboration. I have written this issue not because I expect some action from you (again, being a fellow open source developer, how could I?), but because I wanted to communicate a problem and talk about possible solutions. I know I am not good with words, and I'm sorry about my unnecessary remark, my bad. Me and my student team have collaborated previously with you with some success, and I believe it would be a shame if the collaboration ended over a miscommunication.

As GitHub issues are definitely not the best place to sort out misunderstandings, I'd like to talk with you on any channel you prefer.

@whitequark
Copy link
Member

I accept your apology. I'm happy to discuss this misunderstanding over email, mine is [email protected]. I am also happy to discuss any other delicate organizational matter privately, since it allows for more nuance than a discussion that is exposed to the public and necessarily must be conducted while keeping in mind that others, with no context, will have access to it in perpetuity.


Unfortunately, amaranth-boards as it is now doesn't really serve its purpose, and the people who lose the most are the beginners, who don't really understand everything and want their boards to just work.

I agree in spirit if maybe not in letter. I think it is useful to have because we have quite a few high-quality board definitions, including for some wildly popular open source boards such as iCEBreaker, with some of the lower-quality ones that ended up being merged due to mistakes, lack of attention, and so on. I do agree that these lower-quality definitions present an unnecessary and frustrating obstacle for beginners and I would consider that a significant enough issue that it must have resources diverted to it from other work.

I think you are unusually exposed to the lower-quality platforms because of your position as an educator. (This is a combination of factors that makes the issue more important to resolve!) In general, Intel devices are less popular in the Amaranth community than Lattice or Xilinx devices, and also Intel devices tend to include more unusual features that differ from those in other FPGAs, or featuers that are exposed in an unusual or buggy way. Of course, Terasic is popular in educational settings, especially in the US, creating a "perfect storm" in this case.

I personally also have much less experience with Intel devices than with any other major platform. It would definitely benefit the Amaranth project to have more Intel expertise on board.

Coding style can be checked by flake8, black, or other tools; I use them myself in my projects and find them to be of great help.

In general I have avoided using tools such as flake8 in Amaranth and my other code because I find them of very limited usefulness or unhelpful entirely. However, with amaranth-boards, my opinion is exactly opposite: because the board definitions are designed to be so restricted, any tool that is highly restrictive would be of great benefit if we could make it work. However...

The current syntax (which in and of itself is in need of redesign; one of the bits of Amaranth I'm the least proud of) is not amenable to tools like flake8. You can check this yourself--the Resource syntactic element gets butchered by these tools. This is a problem with the syntax--it is hard for people to write too!--but it still needs to be solved.

Naming and structure could also be (at least partially) verified by a custom checker.

I agree. I also think that most board definitions could be replaced with data in a format equivalent to JSON (but easier to write) that is checked with JSON Schema, like what we are introducing in other parts of Amaranth.

I am quite happy to discuss this or other syntactic changes and I think we can potentially place this on roadmap for Q1 2024.

Checking correctness and completion is a harder problem. It would be best if the reviewer had physical access to the board in question, which is hard because some boards cost quite a lot.

Yes. I have a big stash of boards at my residence that covers most of the amaranth-boards stock, save for some really expensive ones like KC705.

Checking with the board schematics and documentation is in theory sufficient, but in my experience hardware tends to do unexpected things sometimes.

Yes. Also, if you are verifying lots of schematics, you are absolutely bound to make errors purely due to fatigue and the extremely boring nature of the work.

I would imagine that unpaid contributors wouldn't be really interested in doing all of that.

It would perhaps be possible to build a large pool of unpaid contributors who will do this with a low per-head review load, like it happens in the KiCAD libraries. It is unclear whether we can do this, easily or at all. In any case this absolutely requires us to automate everything we can automate to make the job as appealing (or perhaps, as least unappealing) as we can.

So I think that the only way to have reasonable coverage of boards would be to leverage the community: the people interested in having the boards they own supported.

I think this is a good idea but there is a critical question that needs to be addressed. What do we do when a contributor leaves? amaranth-boards is especially prone to drive-by (in this case I'm using it in a strictly neutral sense) contributions, and in some ways this is a strength: it means that a small amount of effort could benefit the community for years to come. However, we do have refactorings of the I/O system planned that will affect the board definitions, and that would likely mean sweeping requests for re-review.

This is a difficult problem to solve and I do not have a satisfactory solution, hence I have not been soliciting such offers of support.

I would imagine that, other than the verified and checked board definitions, there could be a staging area of definitions which passed the immediate checks, but are not sufficiently verified for correctness and completeness. The community could then use these definitions, test them, and point out the issues. Moving a board definition from the staging area to the verified set could be done by accepting a review from a contributor with confirmed credentials.

I'm on board with this. This is by far the easiest part of the proposed plan and I think we can implement it fairly quickly.

I, personally, can offer my time to verify the definitions for the boards I mentioned, DE1-SoC and DE2-115, which I use actively for my classes.

This would be quite welcome and we do not need to wait for anything. I'm happy to review these as soon as you submit them.


Thank you for taking time to submit these proposals, as I do believe that amaranth-boards needs a sustainable way forward and I will do my best to work towards it with others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants