Implementation of relational operator for complex numbers in XLA #39133
Conversation
|
Matching numpy semantics makes sense to me, could we add tests? |
|
@joschkabraun Can you please check @cheshire's comments and keep us posted. Thanks! |
|
Okay, perfect! I’m currently working on it and will finish this weekend! |
tensorflowbutler
removed
the
stat:awaiting response
|
I have been looking through the code. Am I correct in assuming the tests for that should be placed into the file elemental_ir_emitter_test.cc? |
| @@ -1292,6 +1292,16 @@ StatusOr<llvm::Value*> ElementalIrEmitter::EmitComplexBinaryOp( | |||
| llvm_ir::EmitComparison(llvm::CmpInst::FCMP_UNE, | |||
| EmitExtractImag(lhs_value), | |||
| EmitExtractImag(rhs_value), b_)); | |||
| case ComparisonDirection::kGt: | |||
| return Or(llvm_ir::EmitComparison(llvm::CmpInst::FCMP_OGT, | |||
There was a problem hiding this comment.
Could you add a comment that we are using lexicographical comparison which is consistent with numpy behavior?
| @@ -1292,6 +1292,16 @@ StatusOr<llvm::Value*> ElementalIrEmitter::EmitComplexBinaryOp( | |||
| llvm_ir::EmitComparison(llvm::CmpInst::FCMP_UNE, | |||
| EmitExtractImag(lhs_value), | |||
| EmitExtractImag(rhs_value), b_)); | |||
| case ComparisonDirection::kGt: | |||
There was a problem hiding this comment.
What happens to other cases? kLt? Less-or-equal? You could also support this by implementing a normalization pass in algebraic_simplifier
tensorflowbutler
removed
the
stat:awaiting response
|
@joschkabraun Can you please check @cheshire's comments and keep us posted. Thanks! |
tensorflowbutler
added
the
stale
|
It has been 20 days with no activity and the |
|
@joschkabraun, Any update on this PR? Please. Thanks! |
|
Unfortunately, I am currently very busy and seems advisable if someone else could take this over. I am sorry for answering so late! |
google-ml-butler
bot
removed
the
stale
tensorflowbutler
removed
the
stat:awaiting response
|
@joschkabraun Thanks for the confirmation. Closing the PR. |
This is the suggestion to implement relational operators for complex numbers (<, <=, >, >=) as in numpy, i.e., the lexicographical order. In this PR only the ">" relation was implemented as feedback from the community is important to evaluate if this functionality is wished for and makes sense.
This could be leveraged for the implementation of np.unique in JAX as comparison and sorting of complex arrays is not supported by XLA but possible with numpy. Another usage could be an implementation of complex QR decomposition in JAX.
The remaining comparisons for complex numbers and tests for it, will be added if this functionality makes sense.