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

[C++] scalar_if_else_test.cc: The test traces in CheckWithDifferentShapes can be misleading #36824

Closed
felipecrv opened this issue Jul 22, 2023 · 0 comments · Fixed by #36825
Closed

Comments

@felipecrv
Copy link
Contributor

Describe the enhancement requested

The traces might show that a call is being made with a conditional scalar, but then a call is made with an array. If there is a bug in the kernel, it's hard to know which specialization is to blame given the trace.

Component(s)

C++

@felipecrv felipecrv self-assigned this Jul 22, 2023
pitrou pushed a commit that referenced this issue Jul 24, 2023
…in the if-else kernel tests (#36825)

### Rationale for this change

The traces might show that a call is being made with a conditional scalar, but then a call is made with an array. If there is a bug in the kernel, it's hard to know which specialization is to blame given the trace.

### What changes are included in this PR?

A simplification of the `CheckWithDifferentShapes` and addition of a bit more tracing information.

### Are these changes tested?

Yes. The changes are only made to test code and I verified that bugs in the implementation get caught by the new code.

### Are there any user-facing changes?

No.
* Closes: #36824

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 14.0.0 milestone Jul 24, 2023
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
…hapes in the if-else kernel tests (apache#36825)

### Rationale for this change

The traces might show that a call is being made with a conditional scalar, but then a call is made with an array. If there is a bug in the kernel, it's hard to know which specialization is to blame given the trace.

### What changes are included in this PR?

A simplification of the `CheckWithDifferentShapes` and addition of a bit more tracing information.

### Are these changes tested?

Yes. The changes are only made to test code and I verified that bugs in the implementation get caught by the new code.

### Are there any user-facing changes?

No.
* Closes: apache#36824

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…hapes in the if-else kernel tests (apache#36825)

### Rationale for this change

The traces might show that a call is being made with a conditional scalar, but then a call is made with an array. If there is a bug in the kernel, it's hard to know which specialization is to blame given the trace.

### What changes are included in this PR?

A simplification of the `CheckWithDifferentShapes` and addition of a bit more tracing information.

### Are these changes tested?

Yes. The changes are only made to test code and I verified that bugs in the implementation get caught by the new code.

### Are there any user-facing changes?

No.
* Closes: apache#36824

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants