-
Notifications
You must be signed in to change notification settings - Fork 369
Enable Miri tests in CI with non-blocking execution #697
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
base: main
Are you sure you want to change the base?
Changes from all commits
adc532b
cd02489
3b7eeb7
b2b0400
e17c07f
f5f255b
1f8c1de
337a120
a54c9c1
7a97192
9e7d4ae
f288d34
56e0605
7bc3e20
0833183
bf86fef
5a9dc32
a9cdeb7
399c945
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -457,14 +457,15 @@ pub fn lloyds( | |
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use diskann_utils::{lazy_format, views::Matrix}; | ||
| #[cfg(not(miri))] | ||
| use diskann_utils::lazy_format; | ||
| use diskann_utils::views::Matrix; | ||
| use diskann_vector::{distance::SquaredL2, PureDistanceFunction}; | ||
| use rand::{ | ||
| distr::{Distribution, Uniform}, | ||
| rngs::StdRng, | ||
| seq::{IndexedRandom, SliceRandom}, | ||
| Rng, SeedableRng, | ||
| }; | ||
| #[cfg(not(miri))] | ||
| use rand::distr::{Distribution, Uniform}; | ||
| #[cfg(not(miri))] | ||
| use rand::seq::IndexedRandom; | ||
| use rand::{rngs::StdRng, seq::SliceRandom, Rng, SeedableRng}; | ||
|
|
||
| use super::*; | ||
|
|
||
|
|
@@ -477,6 +478,7 @@ mod tests { | |
| // relatively quickly. | ||
| // | ||
| // Outside of rare validations, Miri tests go through a different path for speed purposes. | ||
| #[cfg(not(miri))] | ||
| fn test_distances_in_place_impl<R: Rng>( | ||
| ndata: usize, | ||
| ncenters: usize, | ||
|
|
@@ -556,15 +558,11 @@ mod tests { | |
| } | ||
| } | ||
|
|
||
| cfg_if::cfg_if! { | ||
| if #[cfg(miri)] { | ||
| const TRIALS: usize = 1; | ||
| } else { | ||
| const TRIALS: usize = 100; | ||
| } | ||
| } | ||
| #[cfg(not(miri))] | ||
| const TRIALS: usize = 100; | ||
|
|
||
| #[test] | ||
| #[cfg(not(miri))] | ||
| fn test_distances_in_place() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably skip this test entirely when running under
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot, disable test_distances_in_place() test for miri. revert the changes made in this test.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disabled test_distances_in_place for miri and reverted changes. Commit: 0833183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot, Use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to use #[cfg(not(miri))] for conditional compilation. Commit: 5a9dc32 |
||
| let mut rng = StdRng::seed_from_u64(0xece88a9c6cd86a8a); | ||
| for ndata in 1..=31 { | ||
|
|
@@ -719,12 +717,22 @@ mod tests { | |
| #[test] | ||
| fn end_to_end_test() { | ||
| let mut rng = StdRng::seed_from_u64(0xff22c38d0f0531bf); | ||
| let setup = EndToEndSetup { | ||
| ncenters: 11, | ||
| ndim: 4, | ||
| data_per_center: 8, | ||
| step_between_clusters: 20, | ||
| ntrials: 10, | ||
| let setup = if cfg!(miri) { | ||
| EndToEndSetup { | ||
| ncenters: 3, | ||
| ndim: 4, | ||
| data_per_center: 2, | ||
| step_between_clusters: 20, | ||
| ntrials: 2, | ||
| } | ||
| } else { | ||
| EndToEndSetup { | ||
| ncenters: 11, | ||
| ndim: 4, | ||
| data_per_center: 8, | ||
| step_between_clusters: 20, | ||
| ntrials: 10, | ||
| } | ||
| }; | ||
| end_to_end_test_impl(&setup, &mut rng); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2003,6 +2003,12 @@ mod tests { | |
| let dist = Uniform::new_inclusive(min, max).unwrap(); | ||
|
|
||
| for dim in 0..dim_max { | ||
| // Only run the maximum dimension when running under miri. | ||
| #[cfg(miri)] | ||
| if dim != dim_max - 1 { | ||
| continue; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to be more careful about skipping values here. Unrolling in the distance kernel implementations means that there are a lot of corner cases that need checking which are not necessarily covered by a single test. An alternative is to have an audit and supply a list of specific dimensions to test. |
||
|
|
||
| let mut x_reference: Vec<u8> = vec![0; dim]; | ||
| let mut y_reference: Vec<u8> = vec![0; dim]; | ||
|
|
||
|
|
@@ -2092,7 +2098,7 @@ mod tests { | |
|
|
||
| cfg_if::cfg_if! { | ||
| if #[cfg(miri)] { | ||
| const MAX_DIM: usize = 128; | ||
| const MAX_DIM: usize = 8; | ||
| const TRIALS_PER_DIM: usize = 1; | ||
| } else { | ||
| const MAX_DIM: usize = 256; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1512,6 +1512,11 @@ mod tests { | |
| fn test_binary_dense() { | ||
| let mut rng = StdRng::seed_from_u64(0xb3c95e8e19d3842e); | ||
| for len in 0..MAX_DIM { | ||
| #[cfg(miri)] | ||
| if len != MAX_DIM - 1 { | ||
| continue; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests in |
||
|
|
||
| test_send_and_sync::<1, Binary, Dense>(); | ||
| test_empty::<1, Binary, Dense>(); | ||
| test_construction_errors::<1, Binary, Dense>(); | ||
|
|
@@ -1558,6 +1563,11 @@ mod tests { | |
| fn test_4bit_bit_transpose() { | ||
| let mut rng = StdRng::seed_from_u64(0xb3c95e8e19d3842e); | ||
| for len in 0..MAX_DIM { | ||
| #[cfg(miri)] | ||
| if len != MAX_DIM - 1 { | ||
| continue; | ||
| } | ||
|
|
||
| test_send_and_sync::<4, Unsigned, BitTranspose>(); | ||
| test_empty::<4, Unsigned, BitTranspose>(); | ||
| test_construction_errors::<4, Unsigned, BitTranspose>(); | ||
|
|
||
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.
Can these bounds be set up with something like
and such to avoid the repitition?