int8 and uint8 support in Pad and other ops #387
Conversation
igor-yusupov
changed the title
igor-yusupov
changed the title
.DS_Store
Outdated
There was a problem hiding this comment.
Can you remove this macOS-specific .DS_Store file. I suggest creating a global .gitignore for this.
There was a problem hiding this comment.
Done. Sorry, I accidentally pushed that file. I also updated .gitignore file.
I think we need to make squash commit to not have this file in git history
| Input::Int8Tensor(t) => Ok(t.map_in(pool, |x| *x as f32).into()), | ||
| Input::UInt8Tensor(t) => Ok(t.map_in(pool, |x| *x as f32).into()), | ||
| }, | ||
| DataType::Int8 => match input { |
There was a problem hiding this comment.
I think that using as for casts is fine for all cases here. Just making some notes for myself:
Converting non-i8 types to i8 will truncate in various ways, and the same for u8. The Cast op specs are here. The main cases of interest are:
- Float -> int when out of range: ONNX spec says undefined, so an
ascast is fine - Int -> Int when out of range: ONNX spec says
when OOR, discard higher bits and reinterpret (with respect to two’s complement representation for signed types). For example, 200 (int16) -> -56 (int8).. This matches the behavior ofasin Rust
.gitignore
Outdated
| @@ -12,3 +12,5 @@ dist/ | |||
| # Converted rten ML models | |||
| *.rten | |||
| models/ | |||
|
|
|||
| *.DS_Store | |||
There was a problem hiding this comment.
I think there was a misunderstanding. OS/editor-specific ignores shouldn't be part of this project's .gitignore. Rather they belong in your personal global gitignore file. See the blog post I linked to in the earlier comment for how to set that up.
There was a problem hiding this comment.
I got it. I have updated global gitignore on my machine and restored the .gitignore file in the repository to its original state. Thank you
|
Some notes for myself on float-to-int cast compatibility hazards: The Saturation seems like it would probably be the most useful failure handling and is cross platform. Sticking with |
|
Note for the changelog: This adds u8 and i8 support to:
|
Change description