Arraymancer
Arraymancer copied to clipboard
Create more descriptive error types
Instead of using ValueError
and IndexError
everywhere it is probably more informative to introduce more descriptive error types.
For example:
- IncompatibleShapeError (for tensors that can't be multiplied together)
- UnreachableError (for when/if elif construct that should cover all cases)
- NotImplementedError (for missing features or for proc/method that must be overloaded, e.g.
forward
andbackward
)
@mratsim This looks like something I can do. Is this still a valid issue ?
Yes it's still a valid issue. Here is the target design.
- All exceptions are currently returned in inline procs that begins with "check_".
- All calls must be gated behind
when compileOption("boundChecks")
- Besides I would like to change calls to use 2 lines for readability.
when compileOption("boundChecks"): check_very_long_foo_name(a,b)
- Besides I would like to change calls to use 2 lines for readability.
- "assert" should be replaced by an appropriate exception
- Exceptions should give as much context as possible (input shapes if relevant) so that debugging experience is easier.
- Testing exceptions should be disabled in release mode: https://github.com/mratsim/Arraymancer/blob/78a92a4b3b3d8c931ed82aa385a4a8692e3472c0/tests/tensors/test_accessors.nim#L34-L47
This is mostly done, there might be a few stragglers left.
Now I would like exceptions more descriptive than ValueError
and IndexError
where relevant (Indexing a tensor or shape [2, 3] with [10, 10] should still be an IndexError
).
The exceptions should inherit from Nim core exceptions I think so that generic Nim tools can still work. For example supposing we introduce IncompatibleShapeError
and SliceError
, the first inherits from ValueError
, the second from IndexError
. I'll let you propose your hierarchy.
Also the number of exceptions is growing so it might be easier for you to move them all in a single file. That's fine.
Thanks for tackling this !
I started a separate file as you've suggested. I am also storing the hierarchy of the exceptions in the same file. For now, I am storing it inside utils folder. Just a quick question. Should I include the exceptions file in all the files where custom exceptions occur, or does an initial import in the arraymancer.nim
takes care of that ?
Don't forget to rebase against master. I removed all includes and use imports now. Most of the checks if not all the checks are in a ./private/p_checks
file.
If you find checks that are not there, move them and this way you only have to import your exceptions in the p_checks file.