gpl: modernize density parallelization

[oharboe]

gpl: modernize density parallelization

Replace old OpenMP raw iterators and unchunked bounds in updateBinsGCellDensityArea and updateDensityFieldBin with modern indexed arrays and explicitly enforce schedule(static) across all phases of the loop.

This removes sequential clear bottlenecks per Nesterov iteration and eliminates heavy kmp_flag_64::wait OpenMP spin-wait barriers during density field evaluation because static scheduling forces threads to take the identical bin distribution across all loops without dynamic work-stealing overhead.

Dogfooding Results:
Combined with the #11084 vectorized forces patch, running the end-to-end global placement flow on a 2.4M-net commercial testcase drops runtime by an additional 100 seconds (1.8%), from 5,511s down to 5,411s. Output placement convergence and metrics remain perfectly 100% bit-identical.

[gemini-code-assist]

Code Review

This pull request refactors loops in nesterovBase.cpp to use index-based iteration parallelized with OpenMP (#pragma omp parallel for schedule(static)) instead of range-based or iterator-based loops. The feedback recommends caching the reference to the bins vector returned by getBins() before entering the parallel loops to avoid the overhead of repeated method calls in these performance-critical sections.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request parallelizes several loops in nesterovBase.cpp using OpenMP with static scheduling and index-based iteration. However, a compilation error is introduced in NesterovBase::updateDensityFieldBin because the variable bins is declared twice within the same function scope.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors several loops in nesterovBase.cpp to use index-based iteration parallelized with OpenMP static scheduling instead of range-based or iterator-based loops. The review feedback recommends adding conditional clauses (if) to the OpenMP directives to prevent parallelization overhead and potential runtime issues when the thread count is one or less.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request parallelizes several loops over bins in nesterovBase.cpp using OpenMP with static scheduling. The review feedback highlights potential undefined behavior in OpenMP if the thread count evaluates to zero or a negative number, and suggests using std::max to guarantee a positive thread count for the num_threads clause.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors loops in nesterovBase.cpp to use index-based iteration, allowing for cleaner OpenMP parallelization with static scheduling. The reviewer suggested improving readability and consistency in BinGrid::updateBinsGCellDensityArea by defining a local thread_count variable instead of using a multi-line preprocessor directive.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors several loops in nesterovBase.cpp to use index-based iteration and conditional OpenMP parallelization with a safe thread count. The review feedback points out that performing an OpenMP reduction directly on the class member variable sumPhi_ can lead to compilation errors or undefined behavior on certain compilers, and recommends using a local variable for the reduction instead.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request modernizes OpenMP parallel loops in src/gpl/src/nesterovBase.cpp by replacing old-style iterator loops with index-based loops, adding schedule(static) directives, and using local variables for reductions. Feedback suggests also updating a third loop in updateBinsGCellDensityArea to use index-based iteration and static scheduling to ensure consistency and maximize cache locality.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request modernizes several OpenMP loops in nesterovBase.cpp by replacing iterator-based loops with index-based loops, adding static scheduling, and avoiding parallel region overhead for single-threaded execution. Additionally, it refactors the reduction on the class member sumPhi_ to use a local variable for better portability. The review feedback suggests further optimizing the loop in updateBinsGCellDensityArea by adding an if (num_threads_ > 1) clause and refactoring class member reductions to local variables to prevent potential compilation issues.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request parallelizes bin-area clearing, density updates, and electro-phi updates using OpenMP, introducing local accumulation variables for reduction. The feedback highlights potential floating-point precision loss when accumulating values over a large number of bins using float precision. It is recommended to use double for local accumulation variables (local_sum_overflow_area, local_sum_overflow_area_unscaled, and local_sum_phi) and then cast or round them appropriately when assigning them back to the class member variables.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request parallelizes and optimizes bin-area clearing and density updates in nesterovBase.cpp using OpenMP. It introduces local reduction variables to avoid direct reductions on member variables and replaces iterator-based loops with index-based loops. The reviewer identified an inconsistency in BinGrid::updateBinsGCellDensityArea where the second parallel loop uses num_threads_ instead of the local thread_count variable, which bypasses the caller's thread configuration and can lead to synchronization overhead.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request parallelizes the bin-area clearing loop and refactors OpenMP loops in nesterovBase.cpp to use index-based iteration and local variables for reductions instead of member variables. The feedback suggests using thread_count instead of parallel_threads in the OpenMP if clause of the first loop to maintain consistency with other parallel loops in the file.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request modernizes several OpenMP loops in nesterovBase.cpp by converting iterator-based loops to index-based loops, introducing safe thread-count checks, and specifying static scheduling. It also refactors reduction operations to use local variables instead of directly modifying class member variables within parallel regions. The reviewer suggests adding schedule(static) to one of the parallel loops for consistency and to explicitly enforce the scheduling policy.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors OpenMP parallelization in src/gpl/src/nesterovBase.cpp by replacing iterator-based loops with index-based loops and introducing local variables for reduction operations to safely update member variables. It also ensures thread safety by guarding parallel regions with a validated thread_count. The review feedback suggests improving the readability and consistency of the OpenMP pragmas by reordering and formatting their clauses.

[oharboe]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors OpenMP parallel loops in nesterovBase.cpp to improve thread safety and precision by replacing direct reductions on class member variables with local double accumulators and switching to index-based loops. The reviewer noted a potential precision loss in the calculation of local_sum_phi where casting the bin area sum to float instead of double can introduce truncation errors, and suggested performing the entire calculation in double precision.

[oharboe]

@gudeh @maliberty gemini doesn't converge and it gets things wrong, hence the numerous iteraitons here.

[gudeh]

@LucasYuki has been working with MT related changes in gpl

[oharboe]

@LucasYuki has been working with MT related changes in gpl

Great! My finding here is that there is some performance on the table here. the code is unremarkable.

Should I close this PR?