Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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?

share|improve this question
add comment

1 Answer

up vote 1 down vote accepted

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.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.