Moved basic C error checking to Elixir #20
Conversation
lib/tensorflex.ex
Outdated
| raise ArgumentError, message: "Rows/Columns cannot be less than 1" | ||
| end | ||
| if(empty_list? datalist) do | ||
| raise ArgumentError, message: "Data provided cannot be an empty list" |
There was a problem hiding this comment.
Error messages in Elixir start in lower case. You also don't need the message key. So you should do this:
raise ArgumentError, "data provided cannot be an empty list"
There was a problem hiding this comment.
I apologize for such careless mistakes. Made the required changes in this commit
| @@ -1,8 +1,21 @@ | |||
| defmodule Tensorflex do | |||
|
|
|||
| alias Tensorflex.{NIFs, Graph, Tensor, Matrix} | |||
|
|
|||
| defp empty_list?([[]]), do: true | |||
There was a problem hiding this comment.
Do you need to check if it is a list with an empty list OR if any entries in the list is an empty list?
There was a problem hiding this comment.
I need to specifically check if it is an empty list within another list, that is, [[]]
For all other cases it is fine. For example a user could specify: Tensorflex.create_matrix(1,1,[[5]]) and that would be okay.
lib/tensorflex.ex
Outdated
| @@ -12,11 +25,21 @@ defmodule Tensorflex do | |||
| end | |||
|
|
|||
| def create_matrix(nrows, ncols, datalist) do | |||
| if(nrows < 1 or ncols < 1) do | |||
There was a problem hiding this comment.
You can also do this as guards:
def create_matrix(nrows, ncols, datalist) when nrows > 0 and ncols > 0 do
There was a problem hiding this comment.
This way you don't need to explicitly raise argument error.
There was a problem hiding this comment.
I was thinking of doing that initially, but I thought that it might make it more cryptic for users to understand where they went wrong since they would have to figure out from the condition which fails, what went wrong.
But I think for some of the more obvious cases, like having a float argument for a float tensor, number of rows and columns to be greater than 0 for creating a matrix, we should have guards. I have made these changes in the latest commit.
|
👍 |
I have moved the basic error checking capability to Elixir code because now we can actually obtain the arguments before passing them to the NIFs. I believe I have covered all the scenarios. However, for some functions I haven't explicitly raised errors, and that is because pattern matching will fail and take care of any problem. That is, you can't pass a graph instead of a tensor or a matrix, because the structs are different.
Do you have anything else in mind @josevalim?