0

I'm trying to set the value 1 at each position of a two-dimensional vector (graph) using the crossbeam_utils version 0.8.0. The sub-matrices each thread receives are disjoint. I'm running the code with cargo run on Windows. My code is based on these question1 and question2. But, I'm getting the following error:

thread '<unnamed>' panicked at 'index out of bounds: the len is 3 but the index is 3'

However, if I comment the command (*slice)[row][col] = 1 in the threads spawning, I can see the threads working perfectly as printed in the console.

What can is it causing this error?

use crossbeam_utils::thread;
use std::slice;

const NTHREADS: usize = 2;

fn build_graph() -> Vec<Vec<usize>> {

    let dictionary: Vec<String> = vec!["monk".to_string(), "mock".to_string(), "pock".to_string(),
                                       "pork".to_string(), "perk".to_string(), "perl".to_string()];
    let s_graph = dictionary.len();
    let mut graph: Vec<Vec<usize>> = vec![vec![0; s_graph]; s_graph];
    
    let chunk = f32::ceil(s_graph as f32 / NTHREADS as f32) as usize;
    let mut sliced_graph: Vec<&mut [Vec<usize>]> = Vec::with_capacity(NTHREADS);
    let ptr_graph = graph.as_mut_ptr();

    let mut offset;
    let mut count_of_items;
    
    // Creating mutable references for slices of the matrix
    for n_th in 0..NTHREADS {
        offset = n_th * chunk;
        count_of_items = if offset + chunk > s_graph { s_graph - offset } else { chunk };
        unsafe {
            sliced_graph.push(slice::from_raw_parts_mut(ptr_graph.offset((offset) as isize), count_of_items));
        }
    }

    thread::scope(|scope| {

        let mut n_th: usize = 0;
        let mut min_bound;
        let mut max_bound;

        for slice in &mut sliced_graph {
            
            min_bound = n_th * chunk;
            max_bound = if min_bound + chunk > s_graph { s_graph } else { min_bound + chunk};

            scope.spawn(move |_| {

                println!("thread number: {} - ini: {} - end: {}", n_th, min_bound, max_bound);

                for row in min_bound..max_bound {

                    for col in 0..s_graph {

                        println!("th:{} -->> set row:{} col:{}", n_th, row, col);
                        (*slice)[row][col] = 1;
                    }
                }

                println!("thread number: {} - Finished!", n_th);
            });

            n_th += 1;
        }
    }).unwrap();

    graph
} 

fn main() {
    build_graph();
}

Herohtar
  • 5,347
  • 4
  • 31
  • 41
tnas
  • 510
  • 5
  • 16
  • 1
    The use of unsafe makes me incredibly wary, but here the issue is pretty simple: `n_th` is 0 for the first sub-matrix and 1 for the second, but that doesn't make sense, the two sub-matrices are indexed on [0, 3[ since they're treated as independent array-like of 3x6. – Masklinn Oct 16 '20 at 12:16
  • 2
    And at a fundamental level I think the issue is the reliance on unnecessary indices, if you just iterated your slice, iterated the rows, and set the cells, you naturally wouldn't have indexing issues e.g. https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=57e947428f80eea17249c7d16088231b – Masklinn Oct 16 '20 at 12:22
  • @Masklinn, thank you very much! This is exactly what I've been trying to do. About the use of unsafe, I used it because I've not found another way to run threads over the same structure. BTW, I've learned too much from the code you have shared. Thanks again! – tnas Oct 16 '20 at 12:53
  • Will you answer my question with the code you have shared? It'd be interesting for documenting the solution. – tnas Oct 16 '20 at 12:59
  • Not relevant to your question, but casting `usize` to `f32` and back is a huge red flag. `f32` only has 24 bits of precision (which means things will start breaking at 8-16 million entries). You should always do integer arithmetic with integers. For instance, instead of `f32::ceil(s_graph as f32 / NTHREADS as f32) as usize`, use `(s_graph + NTHREADS - 1)/NTHREADS`. – trent Oct 16 '20 at 16:00
  • 1
    As for iterating without `unsafe`, look at [`chunks_mut`](https://doc.rust-lang.org/std/primitive.slice.html#method.chunks_mut). – trent Oct 16 '20 at 16:26

1 Answers1

1

The solution proposed by @Masklinn:

use crossbeam_utils::thread;
use std::slice;

const NTHREADS: usize = 2;

fn build_graph() -> Vec<Vec<usize>> {

    let dictionary: Vec<String> = vec!["monk".to_string(), "mock".to_string(), "pock".to_string(),
                                       "pork".to_string(), "perk".to_string(), "perl".to_string()];
    let s_graph = dictionary.len();
    let mut graph: Vec<Vec<usize>> = vec![vec![0; s_graph]; s_graph];
    
    let chunk = f32::ceil(s_graph as f32 / NTHREADS as f32) as usize;
    let mut sliced_graph: Vec<&mut [Vec<usize>]> = Vec::with_capacity(NTHREADS);
    let ptr_graph = graph.as_mut_ptr();

    // Creating mutable references for slices of the matrix
    for n_th in 0..NTHREADS {
        let offset = n_th * chunk;
        let count_of_items = if offset + chunk > s_graph { s_graph - offset } else { chunk };
        unsafe {
            sliced_graph.push(slice::from_raw_parts_mut(ptr_graph.offset((offset) as isize), count_of_items));
        }
    }

    thread::scope(|scope| {
        for (n_th, slice) in sliced_graph.iter_mut().enumerate() {
            scope.spawn(move |_| {
                println!("thread number: {} - {:?}", n_th, slice);
                for (row, r) in slice.into_iter().enumerate() {
                    for (col, cell) in r.into_iter().enumerate() {
                        println!("th:{} -->> set row:{} col:{}", n_th, row, col);
                        *cell = 1;
                    }
                }

                println!("thread number: {} - Finished!", n_th);
            });
        }
    }).unwrap();
    graph
} 

fn main() {
    build_graph();
}
tnas
  • 510
  • 5
  • 16