onnx: workaround pow with negative base #2439
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
candle-onnx/src/eval.rs
Outdated
| // HACK: current implementation of (broadcast_)pow cannot handle negative base; | ||
| // this is rarely an issue, but blocked support of silero-vad which uses x^2. | ||
| // Rather than fix pow for all negative base, we choose to handle just x^2. | ||
| if let Ok(2.0) = (|| input1.flatten_all()?.to_scalar::<f32>())() { |
There was a problem hiding this comment.
Rather than handling the case 2. specifically (which indeed seems very hacky) why not handle any "constant" power with powf, this would feel less like a hack and would handle a lot more cases, e.g. square root etc.
There was a problem hiding this comment.
That would require looping through all the tensor values (sort of inline implementation of broadcast_pow). I guess still kind of a hack but if that's preferable I can try that.
edit: ah, not f32::powf but rather candle's Tensor::powf
There was a problem hiding this comment.
ah, it also requires switching on the DType of the tensor, as i32,u32 don't have powf(self, exp: f32) defined, but rather pow(self, exp: i32).
edit: see above
There was a problem hiding this comment.
Yep exactly, it would use Tensor::powf and so work for exponents that are not 2 (and not feel anymore like a hack but rather an optimization for this case).
There was a problem hiding this comment.
I still left a note in case anyone in the future looks at this thinking "why not just use broadcast_pow for both cases?". As for optimization, I'm not sure if it is faster or not, this could end up a performance pessimization if powf isn't using acceleration but broadcast_pow is.
rather than fully defining pow in the cpu backend (as in #2318), this implements a much smaller change which is sufficient to evaluate silero-vad onnx models. Specifically, checking if pow is run with 2.0 exponent, and if so evaluate as simply `x*x` instead of the cpu backend of `e^(2.0 * ln(x))`.
powf correctly handles a negative base.
LaurentMazare
approved these changes
Aug 22, 2024
|
Thanks! |
EricLBuehler
added a commit
to EricLBuehler/candle
that referenced
this pull request
Sep 11, 2024
* Bump the version to 0.6.1. (huggingface#2438) * onnx: workaround pow with negative base (huggingface#2439) * onnx: workaround pow with negative base rather than fully defining pow in the cpu backend (as in huggingface#2318), this implements a much smaller change which is sufficient to evaluate silero-vad onnx models. Specifically, checking if pow is run with 2.0 exponent, and if so evaluate as simply `x*x` instead of the cpu backend of `e^(2.0 * ln(x))`. * PR: use Tensor::powf insead powf correctly handles a negative base. * onnx: support negative index in Gather (huggingface#2440) index_select does not support negative indexing, but this change adds just enough workarounds in onnx to allow evaluating silero-vad models (which make use of negative indices). * silero-vad v5 example (huggingface#2321) * silero-vad v5 example This change adds an example of how to run silero-vad v5 * PR: rename 'vad' to 'silero-vad' * Update README.md --------- Co-authored-by: Laurent Mazare <[email protected]> * Fix for parler-tts, do not add the last slice of padding tokens. (huggingface#2442) * Fix for parler-tts, do not add the last slice of padding tokens. * Support for the mini model. * Add FastViT model. (huggingface#2444) * fix: qwen2 lm_head loading huggingface#2443 (huggingface#2445) Co-authored-by: Yi Xu <[email protected]> * Update cudarc to 0.12. (huggingface#2451) * Update cudarc to 0.12. * Some cudnn tweaks. * FastViT fixes. (huggingface#2452) * correct optional SE layer dimensions. * head_dim instead of num_heads is 32. * update test example output. * MobileCLIP models S1 and S2 (huggingface#2454) * Allow loading images with given std and mean * OpenCLIP text encoder component * Two MobileCLIP models * Clippy fixes. --------- Co-authored-by: Laurent <[email protected]> * Fix FLUX.1 weights (huggingface#2457) * fix FLUX.1 weights * added flux1-dev.safetensors * Clippy fixes for 1.81.0. (huggingface#2461) * Clippy fixes for 1.81.0. * Another fix. * Make Error::msg more in line with anyhow::Error::msg * Add context trait * Even more flexible * Format --------- Co-authored-by: Laurent Mazare <[email protected]> Co-authored-by: shua <[email protected]> Co-authored-by: Jani Monoses <[email protected]> Co-authored-by: ilookee <[email protected]> Co-authored-by: Yi Xu <[email protected]> Co-authored-by: Eugene Hauptmann <[email protected]>
sgrebnov
pushed a commit
to spiceai/candle
that referenced
this pull request
Jan 18, 2025
* onnx: workaround pow with negative base rather than fully defining pow in the cpu backend (as in huggingface#2318), this implements a much smaller change which is sufficient to evaluate silero-vad onnx models. Specifically, checking if pow is run with 2.0 exponent, and if so evaluate as simply `x*x` instead of the cpu backend of `e^(2.0 * ln(x))`. * PR: use Tensor::powf insead powf correctly handles a negative base.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rather than fully defining pow in the cpu backend (as in #2318), this implements a much smaller change which is sufficient to evaluate silero-vad onnx models. Specifically, checking if we can use
Tensor::powfwhich is defined for negative base.