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

fifo: better testing #1762

Merged
merged 7 commits into from
Nov 8, 2023
Merged

fifo: better testing #1762

merged 7 commits into from
Nov 8, 2023

Conversation

anshumanmohan
Copy link
Contributor

@anshumanmohan anshumanmohan commented Nov 7, 2023

This PR adds a slightly more sophisticated testing setup for the FIFO queues.

Before

  • We had a hand-written file fifo.data.
  • We had a hand-written file fifo.expect.
  • We had a hand-written file fifo.py: the eDSL implementation.
  • On testing with runt, we'd automatically:
    • Run fifo.py to get Calyx code.
    • Run this Calyx code against fifo.data.
    • Compare the output against fifo.expect.

After

  • We have a new script fifo_data_gen.py that prints pseudorandom information to populate three lists: commands, values, ans_mem. These contain the formatting info that Calyx expects, and can be piped into a file fifo.data.
  • We have another new script fifo_oracle.py that:
    • Reads a .data-like thing from stdin (the .data thing will have commands, values, and ans_mem, but we care only about the first two).
    • "Runs" the FIFO program using those two lists, and collects the answer in an ans list.
    • Dumps the ans list along with the commands and values to screen, in an .expect-like format.
    • All told, this output can be piped into fifo.expect.
  • The eDSL code fifo.py is unchanged.
  • The testing process is the same as before; we just probably have a better .data file and a .expect file that was created by a clever oracle and not a tired grad student.

What this PR is not doing

Just to be clear, this PR is not running the Python scripts every time in any clever way. The data-generating Python script is seeded with a constant, and after some brute-force searching I've found a constant (5) that generates data that runs the queue through its paces a little. I've run the two new scripts, piped their results into fifo.data and fifo.expect, and committed those new files. The commands I ran were:

python calyx-py/calyx/fifo_data_gen.py > calyx-py/test/correctness/fifo.data

and

python calyx-py/calyx/fifo_data_gen.py | python calyx-py/calyx/fifo_oracle.py > calyx-py/test/correctness/fifo.expect

If you want to try it with new data, you'll need to change the random seed in fifo_data_gen.py, run these two scripts to clobber the .data and .expect files, and then run runt as usual.

@anshumanmohan
Copy link
Contributor Author

anshumanmohan commented Nov 7, 2023

@calebmkim requesting a review from you since we talked about this recently. I think this is actually a teeny bit more sophisticated that the systolic stuff, since I do have the ability to generate new "problems" for myself, and the oracle reads the new problem every time. This is as opposed to "knowing" the problem beforehand. I pipe the oracle's output to an expect file and then leave the testing of the answer to the usual runt script.

Copy link
Contributor

@calebmkim calebmkim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this all lgtm!
Doesn't have to be in this PR, but I can imagine a future version of this that allows you to set MAX_CMDS and ANS_MEM_LEN user command line inputs to the python program.

@anshumanmohan anshumanmohan mentioned this pull request Nov 8, 2023
3 tasks
@anshumanmohan anshumanmohan merged commit 2f18cb0 into master Nov 8, 2023
5 checks passed
@anshumanmohan anshumanmohan deleted the fifo-better-testing branch November 8, 2023 15:56
rachitnigam pushed a commit that referenced this pull request Feb 16, 2024
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.

None yet

2 participants