Skip to content

Working on porting bootcamp to chiseltest#121

Closed
chick wants to merge 14 commits into
masterfrom
port-to-chisel-test
Closed

Working on porting bootcamp to chiseltest#121
chick wants to merge 14 commits into
masterfrom
port-to-chisel-test

Conversation

@chick
Copy link
Copy Markdown
Contributor

@chick chick commented Oct 27, 2020

IN PROGRESS CONVERSION
Questionable import of scalatest in load-ivy.sc
but it seems to work at moment.
Worksheets done: 2.1, 2.2, 2.6

Questionable import of scalatest in load-ivy.sc
but it seems to work at moment.
Worksheets done: 2.1, 2.2, 2.6
2.3 and 2.4
@jackkoenig jackkoenig requested a review from ducky64 November 2, 2020 20:03
Copy link
Copy Markdown
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

Some suggestions inline in the comments, but looks good as a direct translation so far!

It might be useful to clear the cells before committing to GitHub, the output cells are bloating the diff.

Comment thread 2.1_first_module.ipynb
}
],
"source": [
"// Scala Code: Calling Driver to instantiate Passthrough + PeekPokeTester and execute the test.\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment can go away now because of test magic

Comment thread 2.1_first_module.ipynb
"If all `expect` statements are true, then our boilerplate code will return pass.\n",
"\n",
">Note that the `poke` and `expect` use chisel hardware literal notation. Both operations expect literals of the correct type.\n",
"If `poke`ing a `UInt()` you must supply a `UInt` literal (example: `c.io.in.poke(10.U)`, likewise if the input is a `Bool()` the `poke` would expect either `true.B` or `false.B`.\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might also be useful to provide a negative example for each, eg for UInt, "this is distinct from a Scala number, eg 10, which will not work and type-error"

Comment thread 2.1_first_module.ipynb
" c.io.in.poke(1.U) // Set our input to value 1\n",
" c.io.out.expect(1.U) // Assert that the output correctly has 1\n",
" c.io.in.poke(2.U) // Set our input to value 2\n",
" c.io.out.expect(2.U) // Assert that the output correctly has 2\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have an example anywhere of an expect that fails? Because the current system doesn't return a test success / failure value, it might be useful to demonstrate what happens (exception), and maybe also have a side note on integration w/ ScalaTest

Comment thread 2.2_comb_logic.ipynb
" for (i <- 0 until cycles) {\n",
" val in_a = Random.nextInt(16)\n",
" val in_b = Random.nextInt(16)\n",
" c.io.in_a.poke(in_a.U)\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could be a good candidate for abstracting this poke-and-expect operation into a test helper function.

Comment thread 2.2_comb_logic.ipynb
" import scala.util.Random\n",
" val data = Random.nextInt(65536)\n",
" poke(c.io.fifo_data, data)\n",
" c.io.fifo_data.poke(data.U)\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Possibly use Decoupled operations here, eg .enqueue and friends?

Comment thread 2.2_comb_logic.ipynb
" poke(c.io.in_b, in_b)\n",
" poke(c.io.in_c, in_c)\n",
" expect(c.io.out, in_a*in_b+in_c)\n",
" c.io.in_a.poke(in_a.U)\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Potentially a place to introduce a test helper function

@chick
Copy link
Copy Markdown
Contributor Author

chick commented Dec 1, 2020

Closing, this PR has been replaced by #122, #123, #124

@chick chick closed this Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants