Skip to content

Remove redundant syncs - #14

Merged
alexkorovko merged 2 commits into
mainfrom
remove_syncs
Jun 8, 2026
Merged

Remove redundant syncs#14
alexkorovko merged 2 commits into
mainfrom
remove_syncs

Conversation

@alexkorovko

@alexkorovko alexkorovko commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Refactor
    • Reduced unnecessary GPU synchronization in the solver to improve runtime performance and profiling responsiveness.
    • Preserved existing behavior and public interfaces; no changes to exported APIs.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: e0e40604-825f-434a-b337-4e8b3e0bd931

📥 Commits

Reviewing files that changed from the base of the PR and between 79abb83 and e391062.

📒 Files selected for processing (1)
  • cunls/minimizer/gauss_newton_minimizer.cu
🚧 Files skipped from review as they are similar to previous changes (1)
  • cunls/minimizer/gauss_newton_minimizer.cu

📝 Walkthrough

Walkthrough

Removes two explicit cudaStreamSynchronize() calls in GaussNewtonMinimizer (one in InitializeJacobian, one after the linear solve in Minimize) and reformats many function headers and blocks; runtime ordering and data movement are unchanged.

Changes

CUDA stream synchronization optimization

Layer / File(s) Summary
Constructor, Jacobian init, and CSR conversion
cunls/minimizer/gauss_newton_minimizer.cu
Constructor initializer reflowed; InitializeJacobian removes an unconditional stream sync after building triplet sparse structure and documents why it's unnecessary; CSR conversion formatting unchanged behaviour.
Cost and residual/Jacobian computation
cunls/minimizer/gauss_newton_minimizer.cu
ComputeCostAsync/ComputeCost pointer/size and reduction paths reformatted; ComputeResidualAndJacobian signature/usage reflowed without logic changes.
System assembly, scaling, and mapping solution
cunls/minimizer/gauss_newton_minimizer.cu
ApplyColumnScalingToNormalEquations, MapScaledLinearSolutionToTangentStep, and BuildSystem reformatted; triplet→CSR and squared matrix/RHS assembly preserved.
State updates and small helpers
cunls/minimizer/gauss_newton_minimizer.cu
UpdateStates and related declarations reformatted without behavior change.
Convergence evaluation and accept/reject
cunls/minimizer/gauss_newton_minimizer.cu
CheckConvergence and EvaluateAndCheckConvergence reflowed; async compute + single D2H memcpy preserved; AcceptStep/RejectStep condensed.
Minimize loop, solver init, and sync removals
cunls/minimizer/gauss_newton_minimizer.cu
Minimize function formatting adjusted; solver initialization and profiling unchanged; per-iteration linear-solve block no longer calls cudaStreamSynchronize(stream) before leaving the NVTX scope; loop logging reformatted.

🎯 4 (Complex) | ⏱️ ~45 minutes

"I hopped through kernels, quiet and light,
Removing the sync that kept hops from flight.
Streams now follow their ordered way,
Jacobians build, solvers play — no delay.
🐇✨"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Remove redundant syncs' directly and concisely describes the main change: removing unnecessary CUDA stream synchronization calls.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove_syncs

Comment @coderabbitai help to get the list of available commands and usage tips.

@alexkorovko
alexkorovko merged commit 02458e5 into main Jun 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant