-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
Codecov ReportAttention: Patch coverage is
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. |
There was a problem hiding this 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?
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, |
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: