-
Notifications
You must be signed in to change notification settings - Fork 6
[bicoloring] Use Br and Bc directly for the decompression #194
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 14 14
Lines 1635 1733 +98
=========================================
+ Hits 1635 1733 +98 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
It seems that
I only got this error during the benchmarks. |
@gdalle julia> a = Dict{Int,Int}(i => i+1 for i = 1:100)
julia> Base.summarysize(a)
4464
julia> b = Vector{Int}(undef, 100)
julia> Base.summarysize(b)
840 We have a factor 5 in terms of storage. |
The reason why they are dictionaries is that not every color is used for a row, or for a column. But we could probably put zeros as indices for unused colors |
3774330
to
451cb51
Compare
@gdalle I rebased the branch but I will need your help to fix issue with Update: I fixed it. If the type of the matrice that we want to compress is |
Benchmark Results
|
Hot damn! |
On the other hand, in the previous version of the code (the one on |
We also should use random ordering in the benchmarks, otherwise the results will be trivial. |
I just merged the modifications from #198, the upcoming benchmarks will be more realistic |
@amontoison the benchmarks are updated and now we don't see any significant speedups, but we still have the huge slowdowns on some instances. I'm not sure it's worth the memory savings, what do you think? |
Another issue with the benchmarks is that, in the current state, we're unable to pre-allocate |
0d5e732
to
fcf052b
Compare
07205ae
to
ca44a88
Compare
Note that I still need to do an efficient decompression for acyclic bicoloring. |
@gdalle The PR is finally ready for review! |
186b22b
to
6843a14
Compare
0cbd45d
to
8a0668a
Compare
8a0668a
to
54b5bb2
Compare
Use a lazy representation of
Br_and_Br
.The PR can be merged later but I wanted to check if it works to do the decompression like this.
I would like to describe the decompression of bicoloring with this approach is the paper where we directly use
Br
andBc
.