Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JP-3290 Isolate candidate processing into their own pools #8227

Merged
merged 20 commits into from
Feb 21, 2024

Conversation

stscieisenhamer
Copy link
Collaborator

@stscieisenhamer stscieisenhamer commented Jan 26, 2024

Resolves JP-3290

Closes #7688

This PR addresses the runtime issue of association generation. On inefficiency is that the exposure/association comparison checks exposures against associations that are for mutually exclusive candidates. The membership checking is complicated and takes time, slowing the process greatly. The optimization here is to isolate the candidates into their own pools, removing a large fraction of checking that is unnecessary resulting in significant speedup.

The example in the issue, generating associations for 01243 for association candidate ids o002, c1005 & c1000. The issue states "still running after more than a day without completing". Now this situations runs in about 3 hours.

A second optimization implemented involves the membership checking. If a check fails, the constraints of an association need to be restored from their original state. Originally this involved a deep copy of the full association. This has been modified to only copy off the constraint parameters themselves, introducing a factor of 2.5 speed up in the membership checking.

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • Make sure the JIRA ticket is resolved properly

Copy link

codecov bot commented Jan 26, 2024

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

Comparison is base (4cc0ac1) 75.15% compared to head (d5ca6a7) 75.24%.

❗ Current head d5ca6a7 differs from pull request most recent head ad7ea26. Consider uploading reports for the commit ad7ea26 to get more accurate results

Files Patch % Lines
jwst/associations/generator/generate_per_pool.py 33.33% 20 Missing ⚠️
...t/associations/generator/generate_per_candidate.py 92.30% 6 Missing ⚠️
jwst/associations/lib/utilities.py 87.80% 5 Missing ⚠️
jwst/associations/lib/constraint.py 94.28% 2 Missing ⚠️
jwst/associations/main.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8227      +/-   ##
==========================================
+ Coverage   75.15%   75.24%   +0.08%     
==========================================
  Files         470      474       +4     
  Lines       38604    38755     +151     
==========================================
+ Hits        29014    29161     +147     
- Misses       9590     9594       +4     
Flag Coverage Δ *Carryforward flag
nightly 77.35% <ø> (-0.05%) ⬇️ Carriedforward from 557c3fa

*This pull request uses carry forward flags. Click here to find out more.

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

@stscieisenhamer stscieisenhamer marked this pull request as ready for review February 5, 2024 18:54
@stscieisenhamer stscieisenhamer requested a review from a team as a code owner February 5, 2024 18:54
@stscieisenhamer
Copy link
Collaborator Author

Regression

@hbushouse hbushouse added this to the Build 10.2 milestone Feb 21, 2024
Copy link
Collaborator

@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

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

LGTM

@hbushouse hbushouse merged commit 613383f into spacetelescope:master Feb 21, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

asn_generator takes forever
2 participants