-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Unexpected error with Expr.list.sample with n=0 in some rows but not all #16232
Comments
Also it works if the row with a=1 (so n=1) is first. |
I faced with the same problem. Polars version is 0.20.25 |
I'm taking a look at this. |
The implementation devolves to ca.try_zip_and_apply_amortized(n, |opt_s, opt_n| {
match (opt_s, opt_n) {
(Some(s), Some(n)) => s
.as_ref()
.sample_n(n as usize, with_replacement, shuffle, seed)
.map(Some),
_ => Ok(None),
}}) Here's the impl Series {
pub fn sample_n(
&self,
n: usize,
with_replacement: bool,
shuffle: bool,
seed: Option<u64>,
) -> PolarsResult<Self> {
ensure_shape(n, self.len(), with_replacement)?;
if n == 0 {
return Ok(self.clear());
}
let len = self.len();
match with_replacement {
true => {
let idx = create_rand_index_with_replacement(n, len, seed);
debug_assert_eq!(len, self.len());
// SAFETY: we know that we never go out of bounds.
unsafe { Ok(self.take_unchecked(&idx)) }
},
false => {
let idx = create_rand_index_no_replacement(n, len, seed, shuffle);
debug_assert_eq!(len, self.len());
// SAFETY: we know that we never go out of bounds.
unsafe { Ok(self.take_unchecked(&idx)) }
},
}
} Changing Given |
@reswqa I think maybe you wrote the original code? Do you have any insight? |
Documentation for
But the API makes it trivial to clone() ( |
Here is a demonstration of what I believe is unsound behavior of the fn undefined_behavior() {
let mut series = Series::new("a", [1, 2]);
let mut_ref = &mut series;
let addr1 = std::ptr::addr_of!(*mut_ref);
let mut unstable = UnstableSeries::new(mut_ref);
// This won't compile:
// let non_mut_ref = series.clone();
// But we can now have both a mutable reference and a non-mutable
// reference at the same time a different way, which is completely
// unsound:
let non_mut_ref = unstable.as_ref();
let addr2 = std::ptr::addr_of!(*non_mut_ref);
assert_eq!(addr1 as usize, addr2 as usize);
let cloned = non_mut_ref.clone();
// And now we can mutate cloned, at a distance:
let series2 = Series::new("b", [3, 4]);
unstable.swap(&mut series2.array_ref(0).clone());
assert_eq!(cloned.sum::<usize>().unwrap(), 7);
} |
Checks
Reproducible example
Log output
Issue description
Using Expr.list.sample with n=0 in some rows but not all can raise an unexpected error.
Expected behavior
It works for k=1 p=0, and seems to work with p>0. So the sampling with some rows equal to 0 seems the heart of the problem.
Installed versions
The text was updated successfully, but these errors were encountered: