Skip to content

Adopt optimized J-sync across CL code#1357

Open
j-c-c wants to merge 22 commits intodevelopfrom
Jsync_gpu
Open

Adopt optimized J-sync across CL code#1357
j-c-c wants to merge 22 commits intodevelopfrom
Jsync_gpu

Conversation

@j-c-c
Copy link
Collaborator

@j-c-c j-c-c commented Jan 15, 2026

The main focus of this PR is adopting the optimized J-sync code for all CL algos that can use it. This involved swapping the optimized code in for the pre-existing code in the JSync module, validating that outputs were not altered for adopting algos (up to multiplication by -1 on the signs vector), and ensuring that gpu dispatching is working as expected.

Additionally, I ran some experimental recons of 10081 and updated the gallery experiment to be reflective of the recon in the associated publication. I will run the gallery experiment after initial reviews. I also made some optimizations (~4X-5X speedup) to the _estimate_inplane_rotations method which is currently one of the bottlenecks of the Cn algos.

@j-c-c j-c-c self-assigned this Jan 15, 2026
@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

❌ Patch coverage is 97.76119% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.84%. Comparing base (1910274) to head (77e175e).

Files with missing lines Patch % Lines
src/aspire/abinitio/J_sync.py 96.90% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1357      +/-   ##
===========================================
- Coverage    90.86%   90.84%   -0.03%     
===========================================
  Files          135      135              
  Lines        14683    14658      -25     
===========================================
- Hits         13342    13316      -26     
- Misses        1341     1342       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@j-c-c j-c-c changed the title [WIP] Adopt optimized J-sync in other CL code Adopt optimized J-sync across CL code Jan 30, 2026
@j-c-c j-c-c added cleanup Optimization Performance or Resource Optimzation GPU labels Jan 30, 2026
@j-c-c j-c-c requested a review from garrettwrong January 30, 2026 19:38
Copy link
Collaborator

@garrettwrong garrettwrong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Try to hit the non cu code stuff and let me know when that's ready, then I'll try a pass on the cu code.

@j-c-c
Copy link
Collaborator Author

j-c-c commented Feb 5, 2026

@garrettwrong I hit all the non-cu stuff. Marking for re-review.

@j-c-c j-c-c requested a review from garrettwrong February 5, 2026 16:48
@garrettwrong
Copy link
Collaborator

I think this is okay to continue on ... Hopefully you can merge it in soon and then should mainly just be any docstring cleanup and the long form documentation you've been working on to wrap up the reorg.

@j-c-c
Copy link
Collaborator Author

j-c-c commented Feb 10, 2026

I think this is okay to continue on ... Hopefully you can merge it in soon and then should mainly just be any docstring cleanup and the long form documentation you've been working on to wrap up the reorg.

Ok, great. Thanks @garrettwrong. I'll update the branch and move it along.

@j-c-c j-c-c marked this pull request as ready for review February 10, 2026 16:57
@j-c-c j-c-c requested a review from janden as a code owner February 10, 2026 16:57
@j-c-c j-c-c linked an issue Feb 11, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup GPU Optimization Performance or Resource Optimzation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commonline J_sync method unification

2 participants

Comments