-
Notifications
You must be signed in to change notification settings - Fork 5
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
Greedy colouring for decomposition trees of interval graphs and tests #34
base: intervalDecomposition
Are you sure you want to change the base?
Greedy colouring for decomposition trees of interval graphs and tests #34
Conversation
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.
- Could you fix the issue with the CI fail?
- Could you also change your inputs to the decomposition as a Graph<Integer,E> and Map<Integer,Set>?
- And some other more minor stuff
*/ | ||
protected Iterable<List<V>> getVertexOrdering() | ||
{ | ||
System.out.println((Iterable<List<V>>) graph.vertexSet()); |
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.
remove debug output
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.
Done.
/** | ||
* The input graph | ||
*/ | ||
private Graph<List<V>, E> graph; |
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.
Currently we do not use List as the vertex set, since Lists are not really efficient for set structures and using empty lists for the leaves/roots would break the tree structure. Instead we use Graph<Integer, E> and Map<Integer,Set>. Could you change this in your implementation?
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.
Done.
for(List<V> outerVertex:getVertexOrdering() ) { | ||
|
||
//the case of a generalised decomposition tree | ||
/* for (E edge: graph.edgesOf(outerVertex)) { |
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 you need this code? If not, remove it. If yes, remove it and if you will need it, use this commit to get the code.
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.
Yes, you are right. I do not need it.
int maxColourAssigned = Collections.max(asssignedColors.values()); | ||
|
||
|
||
System.out.println(Arrays.asList(asssignedColors)); |
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.
remove debug output
Coloring<Integer> coloring = new DecompositionTreeColour<>(newGraph).getColoring(); | ||
assertEquals(3, coloring.getNumberColors()); | ||
Map<Integer, Integer> colors = coloring.getColors(); | ||
assertEquals(0, colors.get(1).intValue()); |
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 should not assume the color here. Instead test whether vertices differ in they color or have the same color, so that if in a future commit, the algorithm get changed and results another (valid i.e. by renaming) coloring, this tests will also accept the result.
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.
Done. thank you.
Coloring<Integer> coloring = new DecompositionTreeColour<>(newGraph).getColoring(); | ||
assertEquals(3, coloring.getNumberColors()); | ||
Map<Integer, Integer> colors = coloring.getColors(); | ||
assertEquals(0, colors.get(1).intValue()); |
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.
same as in line 87
It would be good to move this into the color package |
b2406e8
to
3542b5a
Compare
3542b5a
to
b46bb2a
Compare
Changed the parameters and incorporated minor changes