-
Notifications
You must be signed in to change notification settings - Fork 188
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
Add first batch of Catch2 tests for DeviceRadixSort #1164
Conversation
All were overflowing to NaN when casting min/max to float.
Adds a matcher that treats `NaN == NaN`, and a bitwise matcher to allow testing for stability when sorting pos/neg zeros.
Also encapsulates memory handling in member functions.
The filtered type lists are now hardcoded as a WAR. Also removed the warning suppression for the unused var in catch2, as local windows builds did not respect the pragma. Now we just hardcode whether FP types are available for each key pass and disable the relevant tests as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI hasn't started, you might need to rebase on main to incorporate recent CI fixes.
const int begin_bit = GENERATE_COPY(0, 1, num_bits / 3, 3 * num_bits / 4, num_bits); | ||
const int end_bit = GENERATE_COPY(0, 1, num_bits / 3, 3 * num_bits / 4, num_bits); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: same motivation as with num_items
:
const int begin_bit = GENERATE_COPY(0, 1, num_bits / 3, 3 * num_bits / 4, num_bits); | |
const int end_bit = GENERATE_COPY(0, 1, num_bits / 3, 3 * num_bits / 4, num_bits); | |
const int begin_bit = GENERATE_COPY(take(2, random(0, num_bits))); | |
const int end_bit = GENERATE_COPY(take(2, random(begin_bit, num_bits))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generating cases where end_bit < begin_bit
, likely due to Catch2 generator lifetime issues -- it doesn't seem to be updating the generator expressions as expected, even with GENERATE_COPY
. I don't think Catch2 expects the values of the captured variables to be changed dynamically like this.
I'll leave this as is for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@allisonvacanti I was not able to reproduce this:
#define CATCH_CONFIG_MAIN
#include <catch2/catch.hpp>
TEST_CASE("Test", "[test]") {
const int num_bits = 64;
const int dummy = GENERATE_COPY(range(0, num_bits));
const int begin_bit = GENERATE_COPY(take(300, random(0, num_bits)));
const int end_bit = GENERATE_COPY(take(300, random(begin_bit, num_bits)));
if (end_bit < begin_bit) {
INFO("begin_bit = " << begin_bit);
INFO("end_bit = " << end_bit);
FAIL("end_bit < begin_bit");
}
}
Do you have a reproducer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC I was using something like this:
const int begin_bit = GENERATE_COPY(take(2, random(0, num_bits)));
const int end_bit = GENERATE_COPY(begin_bit, take(2, random(begin_bit, num_bits)));
to make sure that we'd always cover the begin_bit == end_bit edgecase.
8c9103d
to
3aa5873
Compare
Description
This is part of #86.
This adds basic testing. A future PR will include tests for segmented sort and large inputs (>2^32 items), at which point the old radix sort tests can be removed.