huggingface / huggingface/candle
broadcast_pow is incorrect for `0^0` and neg lhs?
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
Hey I encountered a slight edge case. The current implementation of `broadcast_pow` produces `NaN` when the lhs and rhs values are 0. The correct value should be `1`
Actually, it seems there are multiple situations where the function is producing `NaN` without 0 in either the lhs or rhs. Both of these test in burn produce NaNs these two tensors are producing all `NaN`s in the test we have for burn currently:
```rust
#[test]
fn should_support_neg_values_with_even_power() {
let data = Data::from([[1.0, -1.0, -2.0], [-3.0, -4.0, -5.0]]);
let tensor = Tensor::::from_data(data, &Default::default());
let pow = Data::from([[2.0, 2.0, 4.0], [4.0, 4.0, 2.0]]);
let tensor_pow = Tensor::::from_data(pow, &Default::default());
let data_actual = tensor.powf(tensor_pow).into_data();
let data_expected = Data::from([[1.0, 1.0, 16.0], [81.0, 256.0, 25.0]]);
data_expected.assert_approx_eq(&data_actual, 3);
}
#[test]
fn should_support_neg_values_with_odd_power() {
let data = Data::from([[1.0, -1.0, -2.0], [-3.0, -4.0, -5.0]]);
let tensor = Tensor::::from_data(data, &Default::default());
let pow = Data::from([[3.0, 3.0, 3.0], [3.0, 3.0, 3.0]]);
let tensor_pow = Tensor::::from_data(pow, &Default::default());
let data_actual = tensor.powf(tensor_pow).into_data();
let data_expected = Data::from([[1.0, -1.0, -8.0], [-27.0, -64.0, -125.0]]);
data_expected.assert_approx_eq(&data_actual, 3);
}
```
the output from the test for the burn-candle backend
```
Tensors are not approx eq:
=> Position 1: 1 != NaN | difference NaN > tolerance 0.0010000000000000002
=> Position 2: 16 != NaN | difference NaN > tolerance 0.0010000000000000002
=> Position 3: 81 != NaN | difference NaN > tolerance 0.0010000000000000002
=> Position 4: 256 != NaN | difference NaN > tolerance 0.0010000000000000002
=> Position 5: 25 != NaN | difference NaN > tolerance 0.0010000000000000002
```
to confirm that this does effect candle's broadcast pow here is the code being called for burn
```
CandleTensor::new(
rhs.tensor
.broadcast_mul(&lhs.tensor.log().unwrap())
.unwrap()
.exp()
.unwrap(),
)
```
and here's the current [broadcast_pow](https://github.com/huggingface/candle/blob/6d83d42efb1c8126c4fc34faee3f5a139b09dec6/candle-core/src/tensor.rs#L2593) in candle
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with broadcast_pow in candle-core/src/tensor.rs and the Burn tests should_support_neg_values_with_even_power and should_support_neg_values_with_odd_power. Verify the implementation handles 0^0 and negative bases with even or odd powers without producing NaN, matching the expected tensors shown in the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100