Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stablehlo.cbrt crashes with complex numbers when StableHLO spec supports it #19482

Open
mofeing opened this issue Nov 19, 2024 · 2 comments
Open

Comments

@mofeing
Copy link

mofeing commented Nov 19, 2024

from EnzymeAD/Reactant.jl#291
CC @wsmoses

StableHLO spec for cbrt specifies that it should support complex numbers but when I try to run it, it crashes with the following error:

terminate called after throwing an instance of 'xla::XlaRuntimeError'
  what():  UNKNOWN: <unknown>:0: error: Expected element type in shape to be floating for cbrt operation; got C128.: 
<unknown>:0: note: see current operation: "func.return"(%0) : (tensor<4xcomplex<f64>>) -> ()

MWE

The MLIR code for the MWE is the following:

module {
  func.func @main(%arg0: tensor<4xcomplex<f64>>) -> (tensor<4xcomplex<f64>>, tensor<4xcomplex<f64>>) {
    %0 = stablehlo.transpose %arg0, dims = [0] : (tensor<4xcomplex<f64>>) -> tensor<4xcomplex<f64>>
    %1 = stablehlo.cbrt %0 : tensor<4xcomplex<f64>>
    %2 = stablehlo.transpose %1, dims = [0] : (tensor<4xcomplex<f64>>) -> tensor<4xcomplex<f64>>
    %3 = stablehlo.transpose %0, dims = [0] : (tensor<4xcomplex<f64>>) -> tensor<4xcomplex<f64>>
    return %2, %3 : tensor<4xcomplex<f64>>, tensor<4xcomplex<f64>>
  }
}

You can forget about the transposes since we use them to accommodate Julia's column-major layout to MLIR's row-major layout and are later optimized away.

@mofeing mofeing changed the title stablehlo.cbrt crashes with complex numbers when StableHLO says it should support it stablehlo.cbrt crashes with complex numbers when StableHLO spec supports it Nov 19, 2024
@akuegel
Copy link
Member

akuegel commented Nov 20, 2024

Stablehlo is promising something that is (currently) not supported by XLA:

case HloOpcode::kCbrt: // Complex cbrt is not implemented in either of the

@GleasonK do you know why stablehlo allows complex type? Do we have some wrong documentation about what types are supported somewhere in XLA?
The fix to shape inference (i.e. check for what is actually supported) was done in this PR: #18438

@majnemer
Copy link
Contributor

I hesitated adding a complex variant of cbrt back in the day because the principal and real valued roots disagreed when the input was real. That said, we should just spec it as the principal root:

  r = abs(z);
  theta = atan2(imag(z), real(z));
  root_r = cbrt(r);
  root_theta = theta / 3.0;

  return root_r * (complex(cos(root_theta), sin(root_theta)));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants