add == for vectors

[SobhanMP]

merge on green.
Closes #246

[codecov-commenter]

Codecov Report

Merging #248 (f7cc5c9) into main (dfcc48a) will decrease coverage by 0.02%.
The diff coverage is 82.60%.

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
- Coverage   91.82%   91.80%   -0.03%     
==========================================
  Files          12       12              
  Lines        7307     7330      +23     
==========================================
+ Hits         6710     6729      +19     
- Misses        597      601       +4     

Impacted Files	Coverage Δ
src/sparsevector.jl	94.90% <82.60%> (-0.23%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dkarrasch]

This reminds of our broadcast code. Couldn't we replace the code by all(A .== B) after the size check?

[SobhanMP]

julia> using SparseArrays

julia> a = sprandn(100000, 0.9);

julia> b = sprandn(100000, 0.9);

julia> @time a .== b;
  0.002611 seconds (7 allocations: 879.344 KiB)

i agree that they are similar but that would make it allocating. i don't think we want that.

[dkarrasch]

What I had in mind is something lazy like all(Broadcast.broadcasted(==, Y, Y)), which is allocation-free, but doesn't seem to use any sparse technology. It seems to just iterate via slow getindex.

[dkarrasch]

Do we have tests for this? Including things like equal vectors where one has a stored zero?

[SobhanMP]

i added a test with explicit zero

[SobhanMP]

if you have something specific in mind, do share

[ViralBShah]

Do we really need all the @inbounds? Does it have a noticeable impact here?

[SobhanMP]

@ViralBShah on really large examples, yeah

julia> @benchmark eq1($A, $B)
BenchmarkTools.Trial: 29 samples with 1 evaluation.
 Range (min … max):  168.045 ms … 186.107 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     173.698 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   174.602 ms ±   4.069 ms  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▁     ▁▁▁▁█  ██ ▁ █▁▁ ▁▁▁█  ▁▁   ▁ ▁   ▁  ▁      ▁          ▁  
  █▁▁▁▁▁█████▁▁██▁█▁███▁████▁▁██▁▁▁█▁█▁▁▁█▁▁█▁▁▁▁▁▁█▁▁▁▁▁▁▁▁▁▁█ ▁
  168 ms           Histogram: frequency by time          186 ms <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark eq2($A, $B)
BenchmarkTools.Trial: 23 samples with 1 evaluation.
 Range (min … max):  189.635 ms … 370.411 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     200.284 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   221.056 ms ±  49.187 ms  ┊ GC (mean ± σ):  0.00% ± 0.00%

   █▅                                                            
  ▅███▁██▅▁▁▁▁▅▁▁▁▁▅▅▅▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▅▁▅ ▁
  190 ms           Histogram: frequency by time          370 ms <

[SobhanMP]

i could replace the last two loops with

return all(iszero, view(nonzeros(A), i:nnz(A))) && all(iszero, view(nonzeros(B), j:nnz(B)))

[SobhanMP]

@ViralBShah so do i replace it or no?

[ViralBShah]

I think either way is fine.