vacu9708 opened a new pull request, #18061:
URL: https://github.com/apache/tvm/pull/18061

   # Summary
   I was trying to resolve https://github.com/apache/tvm/issues/18004, where an 
ONNX model causes a segmentation fault in TVM but not in onnxruntime.
   **Why the seg fault occurs**
   This occurs because `take()`(used in `Compress`)defaults to `fast` mode, 
which deliberately segfaults on out-of-bounds indices. (I guess for the sake of 
fast speed)
   **Solution**
   onnxruntime's `Compress` ignores out-of-bounds indices, so I initially 
considered matching that behavior in TVM. However, the ONNX specification 
doesn’t actually mandate how to handle out-of-bounds indices
   **Question for maintainers**
   Would you prefer a follow-up change to `relax.nonzero()`(used by 
`Compress`), adding an optional "max_index" param to slice out invalid indices 
to mirror onnxruntime? although I guess this is unnecessary.
   
   Instead of resolving this seg-fault, for now, this PR focuses just on 
enhancing `take()`.
   
   # Updates
   - Add a `mode` parameter to`take()` in Relax
     - > There are already several modes implemented in the transform layer, 
but Relax’s `take()` defaults to `fast` with no choice. I've exposed all modes 
so future use cases can pick as needed down the road.
   - Add `NaN` mode to `take()`
     - > I was initially adding a `raise` mode like `numpy.take()`, but since 
there are no precedents raising runtime errors in the transform layer, I added 
an `NaN` mode that returns `NaN` for any out-of-bounds index.
   - Add unit tests covering all `take()` modes
     - > The tests for take() modes had been lost during a refactor.
   - Add a warning log for `fast` mode along an axis
     - > take() along a flattened input shows a warning that lets users know 
about the potential seg fault, while `take()` along an axis does not. I think 
both `take()` need to warn about it.
   - Unify default modes in lower layers to `fast` for consistency with Relax
     - > The default mode for `take()` in Relax is currently `fast`, I’ve made 
the lower layers use `fast` mode as well for consistency.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to