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

Improve HessianTracer performance #45

Merged
merged 5 commits into from
May 6, 2024
Merged

Improve HessianTracer performance #45

merged 5 commits into from
May 6, 2024

Conversation

adrhill
Copy link
Owner

@adrhill adrhill commented May 3, 2024

  • Couple key-type of HessianTracer dict to set-type
  • Move sorting kwarg into inner constructor of SortedVector
  • When merging HessianTracers, make a set out of the keys outside of the inner loop. This should avoid repeated sorting for unions with SortedVector.

@adrhill adrhill requested a review from gdalle May 3, 2024 18:43
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 71.34%. Comparing base (e3dca46) to head (cf1d2a0).

Files Patch % Lines
src/tracers.jl 54.54% 20 Missing ⚠️
src/sortedvector.jl 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
- Coverage   73.98%   71.34%   -2.64%     
==========================================
  Files           9        9              
  Lines         319      335      +16     
==========================================
+ Hits          236      239       +3     
- Misses         83       96      +13     

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

src/tracers.jl Show resolved Hide resolved
src/sortedvector.jl Show resolved Hide resolved
src/sortedvector.jl Show resolved Hide resolved
src/pattern.jl Show resolved Hide resolved
src/tracers.jl Show resolved Hide resolved
src/tracers.jl Show resolved Hide resolved
src/tracers.jl Show resolved Hide resolved
src/tracers.jl Show resolved Hide resolved
src/tracers.jl Show resolved Hide resolved
test/benchmark.jl Show resolved Hide resolved
@adrhill
Copy link
Owner Author

adrhill commented May 6, 2024

Merging now and leaving conversations unresolved for a follow-up PR.

@adrhill adrhill merged commit f20d411 into main May 6, 2024
2 checks passed
@adrhill adrhill deleted the ah/sortkeys branch May 6, 2024 18:13
@gdalle
Copy link
Collaborator

gdalle commented May 6, 2024

closed the remaining conversations because I recorded the concerns in #40

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.

3 participants