OpenMP loop parallel for loop with function calls and STL vector

Question

I have a function initialize_path_statistics(). I have used openMP to make it parallel. I am not sure where certain lines such as

float length_enclosed = Nodes::get_length_enclosed(i);

need additional pragma like #pragma omp atomic.

void Path_Statistics :: initialize_Path_statistics()
{    
    path_match_score.resize(no_of_valid_nodes);
    combinatorial_structures.resize(no_of_valid_nodes);
    node_path_matches.resize(no_of_valid_nodes);
    path_loop_indexes.resize(no_of_valid_nodes);
    node_path_energy.resize(no_of_valid_nodes);

    #pragma omp parallel for
    for(unsigned int i = 0;i < no_of_valid_nodes;i++)
    {    
        int matches = node_matches[i];
        float length_enclosed = Nodes::get_length_enclosed(i);
        float match_density = float(matches)/length_enclosed;
        path_match_score[i] = match_density;

        int child_count = (adjacency_table_vector[i].second).size();
        if((adjacency_table_vector[i].second).empty())
        {
            combinatorial_structures[i] = 1;
        }
        else
        {
            combinatorial_structures[i] = child_count+1;
        }   
        node_path_matches[i] = matches;

        int loop_start_position = node_start[i] + node_matches[i];
        int loop_end_position = node_end[i] - node_matches[i];

        path_loop_indexes[i].first = loop_start_position;
        path_loop_indexes[i].second = loop_end_position;

        //the size of the loop
        int loop_size = node_end[i] - node_start[i] - 2*node_matches[i] + 1;

        //if the node can form a valid loop node 
        if(loop_size <= MAXIMUM_SIZE_OF_LOOP)
        {
            //the loop path energy will also have the energy of the node + loop formed
            float energy = get_loop_energy(i) + node_energy[i];
            node_path_energy[i] = energy;
        }
        else
        {   
            node_path_energy[i] = float(INFINITE);

        }

    }

The code for nodes::get_length_enclosed(i) is:

//calculates the length enclosed by the given node
inline int Nodes :: get_length_enclosed(int node_id)
{
    int length = node_end[node_id] - node_start[node_id] +1;        
    return(length);
}

Can I make it more parallel using sections somehow? I tried #pragma omp parallel for sections, which doesn't work. Also, is this code thread safe?

Answer 1

The code seems thread-safe to me. All operations are either done with local variables or with data addressed via the loop index, so data races are seemingly absent. Thus, no additional protection like #pragma omp atomic is necessary.

I do not think you can make it "more parallel". Though technically parts of the loop iteration are independent and can be computed in parallel, the overall amount of computation seems rather small, so exploiting that "inner" parallelism is unlikely to bring more performance. If you want to try it though, I would recommend you to use #pragma omp task, not sections.

Answer 2

Although a bit nitpicky, get_length_enclosed() can just be one line:

return node_end[node_id] - node_start[node_id] +1;

The variable isn't necessary since the function name already reveals its intent.

The inline keyword isn't necessary. The compiler itself will decide if it's worth inlining and can still ignore this hint.

It should also be const since it's is a member function that doesn't modify any data members:

int Nodes :: get_length_enclosed(int node_id) const