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.

Sign up for GitHub

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

Jump to bottom

ARROW-17304: [C++][Compute] Print actual values when compare fails in aggregate test #13814

Merged
merged 2 commits into from
Aug 9, 2022
Merged

ARROW-17304: [C++][Compute] Print actual values when compare fails in aggregate test #13814

merged 2 commits into from
Aug 9, 2022

Conversation

cyb70289
Copy link
Contributor

@cyb70289 cyb70289 commented Aug 8, 2022

No description provided.

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

https://issues.apache.org/jira/browse/ARROW-17304

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

pitrou
pitrou reviewed Aug 8, 2022
View reviewed changes
cpp/src/arrow/compute/kernels/aggregate_test.cc
@@ -2543,7 +2543,9 @@ void CheckModes(const Datum& array, const ModeOptions options,
for (int i = 0; i < out_array.length(); ++i) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could perhaps also add ARROW_SCOPED_TRACE("Mode options = ", options.ToString()) somewhere above to provide better insight into what is computed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@cyb70289 cyb70289 force-pushed the 17304-agg-test-debug branch from 2943422 to ab518db Compare August 9, 2022 04:51
kou
kou approved these changes Aug 9, 2022
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@pitrou pitrou merged commit 709f8f3 into apache:master Aug 9, 2022
@cyb70289 cyb70289 deleted the 17304-agg-test-debug branch August 9, 2022 07:12
@ursabot
Copy link

ursabot commented Aug 9, 2022

Benchmark runs are scheduled for baseline = b83da3f and contender = 709f8f3. 709f8f3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.14% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.27% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.43% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 709f8f33 ec2-t3-xlarge-us-east-2
[Finished] 709f8f33 test-mac-arm
[Finished] 709f8f33 ursa-i9-9960x
[Finished] 709f8f33 ursa-thinkcentre-m75q
[Finished] b83da3fc ec2-t3-xlarge-us-east-2
[Finished] b83da3fc test-mac-arm
[Finished] b83da3fc ursa-i9-9960x
[Finished] b83da3fc ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Aug 9, 2022

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pitrou pitrou pitrou approved these changes

@lidavidm lidavidm lidavidm approved these changes

@kou kou kou approved these changes

Assignees
No one assigned
Labels
Component: C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cyb70289 @ursabot @kou @lidavidm @pitrou