-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: allow neutral colors and postprocessing #166
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 1349 1422 +73
=========================================
+ Hits 1349 1422 +73 ☔ View full report in Codecov by Sentry. |
src/coloring.jl
Outdated
end | ||
end | ||
# remap colors to decrease the highest one by filling gaps | ||
color .= remap_colors(color)[1] |
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.
We could easily check if a neutral color was assigned before calling this function.
We just need to update a counter in the "if condition" line 564.
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.
Do we already know at this step which columns / rows have the same color?
It could help to determine if the neutral color helped to remove a color or just impact the seed.
Talking about the seed, we didn't discussed about that but even if have the same number of colors, having seeds with more zeros (because of rows / columns of neutral color) could not help AD tools where they walk through an expression tree?
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.
We could easily check if a neutral color was assigned before calling this function.
Good idea, I'm doing it right now
Do we already know at this step which columns / rows have the same color?
It could help to determine if the neutral color helped to remove a color or just impact the seed.
The present code doesn't remove a color only from some vertices, it removes it all at once if that color was found to be useless.
Talking about the seed, we didn't discussed about that but even if have the same number of colors, having seeds with more zeros (because of rows / columns of neutral color) could not help AD tools where they walk through an expression tree?
No, the AD tools are agnostic to what's inside the seed. You can think of it as matrix-vector multiplication: doing A * rand(10)
doesn't take more time than A * zeros(10)
@gdalle In acyclic coloring, we can change the color of some columns to the neutral color if they are leaves in certain special trees (trivial trees and stars). |
Co-authored-by: Alexis Montoison <[email protected]>
Good point, this can definitely go in the paper! Functionally, the current code does the same thing, except it is slightly obscured because I chose to stick to the structure of the decompression routine. But I opened #168 to keep track of the idea |
LGTM |
To add to my answer, some vertices could only be leaves of "normal" trees and could be neutralized. However, because other vertices of the same initial colors are internal nodes of the trees, this will not lead to a reduction in the number of colors. I briefly discussed this question of gain with modified seeds (which lead to full rows of zeros in the seed matrix) with Michel Schanen (@michel2323) this morning. |
Fixes #122, fixes #158
Done:
postprocessing
kwarg toGreedyColoringAlgorithm
which allows removing useless colors and replacing them with the "neutral" color0
. This only applies to symmetric or bidirectional colorings.StarSet
andTreeSet
storages:StarSet
TreeSet
constructor, so that it is available for postprocessingstack
ing to handle empty color groupsTo do: