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

Support user-defined set types #31

Merged
merged 9 commits into from
May 2, 2024
Merged

Support user-defined set types #31

merged 9 commits into from
May 2, 2024

Conversation

adrhill
Copy link
Owner

@adrhill adrhill commented May 2, 2024

Closes #23 by supporting all sets of type AbstractSet{<:Integer}.
Breaking since the current pattern API now requires the user to specify the set type in the tracer:

julia> using SparseConnectivityTracer

julia> x = rand(3);

julia> f(x) = [x[1]^2, 2 * x[1] * x[2]^2, sin(x[3])];

julia> pattern(f, JacobianTracer{BitSet}, x) # <--- Set type needs to be specifies
3×3 SparseArrays.SparseMatrixCSC{Bool, UInt64} with 4 stored entries:
 1  ⋅  ⋅
 1  1  ⋅
 ⋅  ⋅  1

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2024

Codecov Report

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

Project coverage is 70.49%. Comparing base (b1f2ec3) to head (76f9d7c).

Files Patch % Lines
src/tracers.jl 57.77% 19 Missing ⚠️
src/conversion.jl 36.84% 12 Missing ⚠️
src/overload_jacobian.jl 27.27% 8 Missing ⚠️
src/overload_hessian.jl 40.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
- Coverage   76.32%   70.49%   -5.83%     
==========================================
  Files           8        8              
  Lines         245      261      +16     
==========================================
- Hits          187      184       -3     
- Misses         58       77      +19     

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

Copy link
Collaborator

@gdalle gdalle left a comment

Choose a reason for hiding this comment

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

You were right it's too complex for me to review so I just flagged a few things that stood out to me as abnormal

What is the point of these reference tests if the reference files are changed automatically?

test/references/show/ConnectivityTracer.txt Outdated Show resolved Hide resolved
test/references/show/HessianTracer.txt Show resolved Hide resolved
test/references/show/JacobianTracer.txt Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@adrhill adrhill merged commit 8f9bfa3 into main May 2, 2024
3 checks passed
@adrhill adrhill deleted the ah/set-types branch May 2, 2024 21:58
@adrhill
Copy link
Owner Author

adrhill commented May 2, 2024

To highlight the difference in performance on large scale problems:

julia> x = rand(220, 220, 3, 1); # WHCN

julia> w = rand(5, 5, 3, 8); # Conv((5, 5), 3 => 8)

julia> @time pattern(x -> conv(x, w), JacobianTracer{BitSet}, x)
 57.282859 seconds (141.11 M allocations: 655.483 GiB, 57.00% gc time, 0.14% compilation time)
373248×145200 SparseMatrixCSC{Bool, UInt64} with 27993600 stored entries:
...

julia> @time pattern(x -> conv(x, w), JacobianTracer{Set{UInt64}}, x)
 20.156197 seconds (303.10 M allocations: 45.897 GiB, 14.60% gc time, 0.44% compilation time)
373248×145200 SparseMatrixCSC{Bool, UInt64} with 27993600 stored entries:
...

For small and medium size problems, BitSet is more performant.

@adrhill adrhill mentioned this pull request May 2, 2024
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.

Let user select set-type used in tracers
3 participants